-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 ability to toggle bullets on and off for Latest Posts Block via new "Layout" option. #32806
Add ability to toggle bullets on and off for Latest Posts Block via new "Layout" option. #32806
Conversation
Size Change: +2.02 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
Ok now when we're adding this option, it makes me think that maybe we should make it a dropdown instead, so that users can choose different list styles between I'm just thinking out loud here 😅, this is way beyond the scope of this PR. |
Hi there! thanks for the PR. I think we should be careful about adding customizations options in adhoc way for blocks. It's unclear to me what are we trying to solve here? I think showing the bullets has been considered a theme CSS style historically and not something to tweak manually. So the important thing for me is that for "empty theme" these blocks look the same in the front-end and the editor and when themes provide styles, they adapt. |
That's fine. Should have been a "Draft" really. More a PoC.
In We could argue that bullets should be shown by default and that Themes should enqueue styles to remove the bullets if they want. Maybe that is all around a better option. However, in this model we fail to consider context. The block has multiple configurations to decide what level of content you want to show:
Therefore a blanket CSS rule to govern display of bullets might not work as users will have varying opinions on when they feel it is appropriate to show bullets or not. OptionsPerhaps adding this attribute is too niche. Fair enough. I see the options available as follows:
Also to be clear, I'm not heavily invested in any of these solutions. |
Worth noting in this discussion also that this block is kind of considered "deprecated" or at least replaced slowly by a more flexible "Query" block. |
How about we add "List" as one of the options underneath Styles and, when selected, this shows the bullets (by styling This makes sense to me as, in my mind, there's two distinct kinds of "Recent Posts" that a user could want:
(It's arguable that 2. should be deprecated entirely in lieu of the Query block.) |
I like that last proposal because of the small impact and potential changes in the future. Let's get some design input. |
I agree. I'll leave this as draft until we get some design input (hat tip @draganescu). |
Hey all, my two cents is that I think it would be confusing to add "list" as a style given that there are already list and grid display options in the toolbar. From a design perspective, it would probably make sense to combine this variation with the existing options. The existing list icon isn't really doing us any favors here but I imagine it could work something like this: |
In general it would be nice to have an approach that can also be used here: I suggested a similar design method as @critterverse suggested above. By adding options directly into the toolbar. |
e2e9642
to
f606f7a
Compare
@critterverse I've updated the implementation as you suggested. Would you be able to review? 🙇♂️ |
This works great! One thing I spotted that's a bit weird is when you enable the "Display featured image" option with the new bullet layout option: Can we adjust this so that the bullet point stays aligned with the title whether or not there's an image? Much more minor note but I also noticed that the bullet point is not visible in the editor when the block is set to full width or wide width (testing with Twenty Twenty-One), althought it does display on the front end. This could be slightly confusing to users if they aren't seeing any visual difference between the 2 options: bullets-twenty-twenty-one.mov^ Not sure what to suggest there because the treatment of the bullet point as "hanging punctuation" is really nice and I wouldn't want to lose that! |
fd780d8
to
b9850a5
Compare
@critterverse I wasn't able to replicate this. Can you advise under what conditions this occurs? I was using TT1 theme. Screen.Capture.on.2021-06-24.at.09-22-37.mov
I've "fixed" this by using
I understand. I also agree it looked nice. However, to have no bullet points show up at wide alignments would be confusing. We have 3 options:
Would you be ok to give the PR a spin as it is currently (ie: |
See also #32959 for the Latest Comments block. |
Same. I just mean like this:
Got it, this makes sense. Option number one works well, I was mostly concerned about the way the text would wrap but it's still behaving like hanging punctuation here so that works: ^ On a similar note as the featured images, can we make sure that the bullet stays aligned with the first row of the title if the text wraps? I can take a look at the Latest Comments block tomorrow! |
I just tested, and I can't see the bullet styles in the customizer. Twenty Twenty also seems to override them in the standalone editor, but that can possibly be considered a theme issue. |
We're not going to try and use this route for solving the Widgets Editor styles for 5.8 as #32983 is a simpler route. Let's still pursue to see if it's helpful for users. |
I won't be pursuing this so closing unless anyone else wants to take over. |
Description
Related #32718.
This PR allows a user to
turn bullet points on and off for the blockto decide between between the following block layouts:This is in response to designer guidance from @critterverse in #32806 (comment).
We will need to replicate this for Latest Comments - probably in the separate PR.See also #32959 for the Latest Comments block.
How has this been tested?
Layout
item in the Block Toolbar.Also check we don't need a deprecation:
trunk
.Screenshots
Screen.Capture.on.2021-06-23.at.16-35-23.mov
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.js
files for terms that need renaming or removal).