Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new RSS Block #7966

Merged
merged 49 commits into from
Jan 25, 2019
Merged

Add new RSS Block #7966

merged 49 commits into from
Jan 25, 2019

Conversation

Soean
Copy link
Member

@Soean Soean commented Jul 14, 2018

Description

New Block to display RSS Feeds.

Fixes #1799

Features:

  • Add Feed URL
  • Select number of items
  • Display author
  • Display date
  • Display excerpt
  • Select except length
  • List and column layout
  • Select column count

Screenshots

jul-15-2018 13-02-58

Placeholder
bildschirmfoto 2018-07-15 um 13 14 11

Add URL https://wordpress.org/news/
bildschirmfoto 2018-07-15 um 13 14 52

Default settings
bildschirmfoto 2018-07-15 um 13 15 04

Enable date, author and except (20 words)
bildschirmfoto 2018-07-15 um 13 15 40

Enable grid layout
bildschirmfoto 2018-07-15 um 13 15 57

Settings with grid layout
bildschirmfoto 2018-07-15 um 13 15 53

Parsing error (Only in backend)
bildschirmfoto 2018-07-15 um 14 05 44

@Soean Soean added [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress New Block Suggestion for a new block labels Jul 14, 2018
core-blocks/rss/index.php Outdated Show resolved Hide resolved
@Soean Soean changed the title WIP: Add new RSS Block Add new RSS Block Jul 15, 2018
@Soean Soean added Needs Design Feedback Needs general design feedback. and removed [Status] In Progress Tracking issues with work in progress labels Jul 15, 2018
@Soean Soean mentioned this pull request Jul 15, 2018
This was referenced Jul 15, 2018
@karmatosed
Copy link
Member

@Soean could you refresh this please so a design review can happen?

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise it looks good. All my comments were addressed. Great work @Soean on RSS block. Thanks for updating PR instantly after all feedback received.

@mapk there is agreement that error state needs to get some improvement. There are some technical challenges which we need to look at first to get it done. Given that we still have more than a week until next planned Gutenberg release, I would suggest we merge this PR in the current shape and improve UI separately.

@gziolo
Copy link
Member

gziolo commented Jan 24, 2019

There is also possible improvements to tackle in the follow-up PR.

  1. Set the minimum number of words to 1 or something higher. 0 doesn't look good:

screen shot 2019-01-24 at 14 06 42

  1. It would be good to clarify that the length of excerpt is the number of words displayed:

screen shot 2019-01-24 at 14 08 15

  1. When there is an error we still display all controls (it's related to the aforementioned change to the way ServerSideRender component works:

screen shot 2019-01-24 at 14 09 36

@melchoyce
Copy link
Contributor

I'm cool with addressing the error messages in a follow-up PR. Can we get a list of all the strings in that future PR?

@mapk
Copy link
Contributor

mapk commented Jan 24, 2019

Given that we still have more than a week until next planned Gutenberg release, I would suggest we merge this PR in the current shape and improve UI separately.

LGTM

I agree about all your additional improvements, @gziolo as well.

  1. Set min number of words.
  2. Let's change text from "Max length of the excerpt" to "Max number of words in excerpt"
  3. Fix the error rendering

and I have a 4th as Mel mentioned above.

  1. Let's fix the error message. I don't think many will understand "WP HTTP Error: cURL ..." Can we just say, "Could not resolve host: xxxx"?

@youknowriad youknowriad dismissed mapk’s stale review January 25, 2019 09:08

Looks like Mark approved :)

@youknowriad
Copy link
Contributor

🎉 Awesome work everyone involved here. Let's get this in and iterate.

@youknowriad youknowriad merged commit be9870d into master Jan 25, 2019
@youknowriad youknowriad deleted the add/rss-block branch January 25, 2019 09:09
@Soean Soean added the [Block] RSS Affects the RSS Block - used to display entries from an RSS/Atom feed label Jan 25, 2019
@gziolo gziolo removed the Needs Design Feedback Needs general design feedback. label Jan 25, 2019
@gziolo gziolo mentioned this pull request Jan 25, 2019
5 tasks
@gziolo
Copy link
Member

gziolo commented Jan 25, 2019

I collected all the issues raised and opened #13498. Feel free to update if I missed anything.

daniloercoli added a commit that referenced this pull request Jan 25, 2019
…rnmobile/372-add-enter-key-detection-to-title

* 'master' of https://github.com/WordPress/gutenberg:
  Add new RSS Block (#7966)
  Performance: optimize the usage of getBlockIndex (#13067)
@melchoyce melchoyce mentioned this pull request Jan 28, 2019
@afercia afercia added the [Feature] Widgets Screen The block-based screen that replaced widgets.php. label Jan 30, 2019
@markzahra
Copy link

Hey folks, not sure if this is the right place to ask (if not, let me know where would be best)...

What was the main reason for looking into adding this block to WordPress Core back in 2017?

I ask as the leader of the current team behind WP RSS Aggregator. I just came across this issue as we've been working on introducing our own Gutenberg block. Essentially, our free core version's block does exactly the same thing as this block, then the rest of our features and add-ons take it a step or two further.

I'm wondering what the future holds for this Gutenberg block to see if and how it may impact our plugin going forward. I've read the related Github issues but couldn't find what actually created the need for this feature to be in WordPress core. It would be great if I could hear some feedback on that and the future plans for the RSS block.

@melchoyce
Copy link
Contributor

Hey @markzahra, this block is part of an effort to convert all existing core widgets into blocks. You can read more about that here: https://make.wordpress.org/core/2018/12/17/status-update-porting-widgets-to-blocks/

I don't think we'll do much beyond what's in this PR, except for maintenance.

@markzahra
Copy link

Thank you for the clarifications @melchoyce, I appreciate it. Looking forward to learning more about the new possibilities with Gutenberg through your work on this block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] RSS Affects the RSS Block - used to display entries from an RSS/Atom feed [Feature] Blocks Overall functionality of blocks [Feature] Widgets Screen The block-based screen that replaced widgets.php. Needs Accessibility Feedback Need input from accessibility Needs Copy Review Needs review of user-facing copy (language, phrasing) New Block Suggestion for a new block
Projects
None yet
Development

Successfully merging this pull request may close these issues.