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

Conversation

@Soean
Copy link
Member

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 added some commits Jul 14, 2018

Show resolved Hide resolved core-blocks/rss/index.php Outdated

@Soean Soean changed the title WIP: Add new RSS Block Add new RSS Block Jul 15, 2018

@Soean Soean referenced this pull request Jul 15, 2018

Closed

Add Block: RSS #1799

Soean added some commits Jul 16, 2018

@sarahmonster sarahmonster added this to New blocks to review in The Great Block Audit of 2018 Jul 31, 2018

@karmatosed

This comment has been minimized.

Copy link
Member

karmatosed commented Oct 8, 2018

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

Soean added some commits Oct 8, 2018

@Soean

This comment has been minimized.

Copy link
Member Author

Soean commented Jan 24, 2019

Already fixed

@gziolo

gziolo approved these changes Jan 24, 2019

Copy link
Member

gziolo left a comment

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

melchoyce commented Jan 24, 2019

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

This comment has been minimized.

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"?

Looks like Mark approved :)

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Jan 25, 2019

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

@youknowriad youknowriad merged commit be9870d into master Jan 25, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

The Great Block Audit of 2018 automation moved this from To do to Done Jan 25, 2019

@youknowriad youknowriad deleted the add/rss-block branch Jan 25, 2019

@Soean Soean added the [Block] RSS label Jan 25, 2019

@gziolo gziolo referenced this pull request Jan 25, 2019

Open

Improve RSS Block #13498

2 of 5 tasks complete
@gziolo

This comment has been minimized.

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

Merge branch 'master' of https://github.com/WordPress/gutenberg into …
…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)

@gziolo gziolo moved this from In Progress to Done in Porting widgets to blocks Jan 28, 2019

@melchoyce melchoyce referenced this pull request Jan 28, 2019

Closed

Add Block: Calendar #1462

@afercia afercia added the Widgets label Jan 30, 2019

@markzahra

This comment has been minimized.

Copy link

markzahra commented Feb 4, 2019

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

This comment has been minimized.

Copy link
Contributor

melchoyce commented Feb 4, 2019

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

This comment has been minimized.

Copy link

markzahra commented Feb 4, 2019

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