xwp / material-design-wp-plugin

Material Design plugin for WordPress
13 stars 6 forks source link

Block: Recent Posts #14

Closed jwold closed 3 years ago

jwold commented 4 years ago

Feature description

Displays a list of the most recent posts from your site. The posts are ordered inside of cards, and have styles to change the display order.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Adding the block

Previewing the block

Block settings

Prototype: Screen Capture on 2020-02-17 at 12-35-36

Figma link

Styles SVGs.zip

Icon description-24px.svg.zip

Implementation brief

QA testing instructions

Note 1: With most of these changes you'll also want to see if the frontend is working, and not just the Gutenberg preview. It's possible there are discrepancies.

Note 2: For now we're just testing with the theme set in the staging env

Adding the block

Previewing the block

Demo

Changelog entry

jwold commented 4 years ago

AC and prototype have both been updated.

davidlonjon commented 4 years ago

@jwold

I think that similarly to the Latest Post core block we will need a rendering for the when there are no posts found such as shown in the screenshot below from the Latest Post core block: image.

Also note that while the post are fetched in the backend, instead of the "No post found" message, the Latest Post core block is showing a spinner.

jwold commented 4 years ago

@davidlonjon if I'm understanding correctly you need two statuses to show for this block. Please let me know if any changes are needed. Thanks!

Screenshots

Screen Shot 2020-02-18 at 8 40 13 AM

Figma https://www.figma.com/file/cNKMIkWy3I0YQBZ6GpSpNG/MDC-Web?node-id=536%3A10931

Dashicon https://developer.wordpress.org/resource/dashicons/#sticky

jwold commented 4 years ago

@davidlonjon here are the new SVGs for the masonry/list/grid styles.

Thanks!

SVG Styles.zip

jwold commented 4 years ago

@davidlonjon can you write the implementation brief for this? Thanks!

davidlonjon commented 4 years ago

@jwold, I have now added the implementation brief

jwold commented 4 years ago

Great work on the demo David! Got some feedback from the team.

davidlonjon commented 4 years ago

Hi @jwold ,

Thank you for the feedback. Let me address them:

Links in new window: I’m torn on this. We could definitely add an option in the block settings, but I don’t know if it’s needed for now.

Yes i think we can chose later on adding a control. For the time being we can open in same window by default.

Links on frontend: In the Video David asked what actions a user could take with links on the fronted. Any buttons in action bar should go to their pages (author to author page, comments to bottom of page with comment section highlighted)

@ravichdev, since you have been working on the frontend, you can make sure of the implementation mentioned by Joshua?

Author and comments: In the video David asked whether these should stay in the action bar for Masonry/Grid. I agree that they should.

Great no changes here then.

List option: In the video David mentioned removing Post Content for List, I agree.

Great no changes here then

Grid option: I’d suggest we can keep it for now, and get user feedback.

Great no changes here then

Featured image size: There’s a new dropdown for changing the featured image size. My concern is that, since it doesn’t change the actual image dimensions, but rather selects a different image to fit within the same dimensions, that it could cause confusion. My suggestion would be to choose a smart default (perhaps a larger size for now), and then programmatically adjust based on viewport size in the future. Would love feedback on this though.

Ravi implemented smart default on the frontend rendering. I have also done it now on the editor and removed the control from the panel. The default are the medium image size for the list layout and the large image size for the masonry and grid layout. We can later think about image size based on viewport size.

Style of rendering: In the video David talked about the CSS styling for the block, and whether it should adhere more to the WordPress Material Theme in all cases, or whether it should adopt another theme (Twenty Twenty for example), when that theme is activated. The general consensus was to stay with Material theme properties for now, and then we can get user feedback after going live.

Agreed. We have done some tweaking to ensure Material theme enforcement for the blocks. @ravichdev, I have changed the font to emin the styles on my PR as well as adding a base size (16px) for the parent of single post cards.

Width: In the video David asked whether the width should be fluid/fixed. I’m inclined to stick to the block contextual toolbar defaults, if they’ll work: https://d.pr/i/sZhMGf

I have implemented it. See the video demo https://cloudup.com/c4yfEHTlifj @ravichdev, I have added the alignment control in the toolbar. It means that the block should inherit of a class to allow control alignment on the frontend.

Text placement: We discussed in the demo video about adding some variants as options. Is this something that could be done for the initial MVP? https://d.pr/i/sVKJo4. Thinking for Grid and Masonry.

I have implemented it. See the video demo https://cloudup.com/c4yfEHTlifj

jwold commented 4 years ago

Great work. This looks amazing! So excited when I saw this demo 😀

I spotted just one thing, can you add a gray border to the styles?

Screen Shot 2020-02-25 at 6 09 28 AM
davidlonjon commented 4 years ago

@ravichdev, The change mentioned above is touching the ImageRadioControl component. Should we do create another issue for it or implement here?

ravichdev commented 4 years ago

@davidlonjon As this is a minor change I think we can implement that as part of this issue.

davidlonjon commented 4 years ago

@jwold , I have now implemented your latest feedback regarding adding the border to the styles.

image

cc @ravichdev

jwold commented 4 years ago

Great work! This looks good 😀

jwold commented 4 years ago

Quick question. Is this the expected behavior when I go to align-center with a single post? https://d.pr/i/xyOjV1. cc @ravichdev and @davidlonjon

davidlonjon commented 4 years ago

Hi @jwold,

We did not defined the expectation and we overlooked this feature from Gutenberg in the editor.

It seems to me there is a bit of inconsistency with this feature as the center alignment has 2 behaviour, it does center the block itself and apply centering to the text inside the block.

We can decide how we want to handle each scenario above. We might not want to do the text centering for example.

Finally there is the other issue when having only 1 posts in a multi column layout. would we want in the case of a center align, the single post to center? In my view no as the single post should be in its column even though there are no other posts.

My view I think the behaviour inside the editor we should only support wide and full width alignment for this block (and the hand picked posts block)

On the frontend this would be up to the theme how to handle the alignment that the editor defines.

Finally I think this is an issue which will be impacting all the block supporting the alignment feature and we might want to create a new issue for it. We should decide which block will support or not alignment and it they do, which type of alignments should be allowed from the 5 default options (left, right, center, wide, full).

Does it make sense?

cc @ravichdev, @emeaguiar , @dugajean

jwold commented 4 years ago

We might not want to do the text centering for example.

I think you're right. Unless there is a precedent where Material supports a center aligned card. I haven't seen that yet though.

Finally there is the other issue when having only 1 posts in a multi column layout. would we want in the case of a center align, the single post to center? In my view no as the single post should be in its column even though there are no other posts.

In looking over this again I agree, you're right. If someone has 2-3 columns it wouldn't make sense to stretch a single card across those columns.

My view I think the behaviour inside the editor we should only support wide and full width alignment for this block (and the hand picked posts block)

Agreed.

Finally I think this is an issue which will be impacting all the block supporting the alignment feature and we might want to create a new issue for it.

I'm happy to do that.

davidlonjon commented 4 years ago

Hi @jwold, specifically for the Recent Posts and Hand-picked Posts blocks, I have created the following issue #134 for these 2 blocks to support only the wide and full alignment. I am also proposing to make wide the default alignment. Finally for implementation I created this PR #135

jwold commented 4 years ago

@csossi this is ready for QA testing. Note that the width of the block is still in progress, per David's comment above ^

davidlonjon commented 4 years ago

Hi @jwold , @csossi ,

Please note that the PR for the alignment issue has been merged and deployed, however it will still not show on the test environment because our material theme may not support the core alignment controls yet. Here is a demo demonstrating the issue on my local https://cloudup.com/cI6_CGe7QT4

We discussed it on Monday with @emeaguiar who created a new issue on Github to fix it. Mario please let me know if there will be anything I'll need to do on the block itself.

jwold commented 4 years ago

Thanks for the update David!

emeaguiar commented 4 years ago

I have a PR in the theme repo to enable alignment support: xwp/material-theme-wp#44 it'll be visible in the testing site once it's merged.

csossi commented 4 years ago

@jwold - alignment seems to offer only Full and Wide widths (nothing for Normal): image

Further, as in the image above, Wide is "Active". It appears I can get to Normal if I click Wide (behaves as a toggle turned "off"). Now, it looks like I'm seeing a Normal layout: image

The prob I'm having here is that the Block toolbar has disappeared. For it to return, I have to click outside of the block and then back in.

davidlonjon commented 4 years ago

Hi @csossi,

Thank you for the feedback.

We took a decision for this block (As well the Hand-picked posts and Card collections blocks) to only have the Wide and Full Screen alignment. I believe that with WordPress there is no "normal" alignment and only by toggling off the selected alignment, it will just no set any alignment. There is left/right/center alignments but they did not really make sense in the context of these blocks because they were impacting the content of the individual cards themselves rather than the container. Surely we can revisit them if there are some expectations from a UX point of view.

Also the second issue (the Block toolbar disappears after unselecting the alignment), it seems to be a core WordPress feature (or bug ?) as it seems to behave the same with the core blocks such as the Image block for example. Is anybody else seeing the same?

cc @jwold @ravichdev

jwold commented 4 years ago

Issue one: Just wanted to followup on this wide/normal/full width discsusion. I may have written the AC incorrectly, but as far as I can tell the block is functioning as expected.

When you first add the block it defaults to wide view, then you can switch back to "normal" view by de-selecting it: https://v.usetapes.com/JptEFTS7u1. This has always been confusing to me that Gutenberg has three options hidden in two buttons, but I think it's ok to leave as is since this is Core functionality.

Issue two: I tried finding another Core block to compare against, but can't verify if this is working as expected. It feels like a bug, but I'm not sure. Ha!

csossi commented 4 years ago

@davidlonjon @jwold

Thanks for this, guys. I checked another WP installation and saw that the Core blocks seem to have the "dissapear" happening there as well, so it seems to behave the same in Core

Ticket verified here