-
Notifications
You must be signed in to change notification settings - Fork 58
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
Refine selected block borders #1802
Comments
That's the correct values and in the code it looks almost the same. In the Base on that each block receive proper class with already calculated value of margin. The cool thing about that are:
If we use the same for all blocks we can be able e.g. to adjust margins across the app with very small effort (only by changing few variables mentioned above)
I remember during work on navigation down border style I ask we should apply the same style for all blocks. My opinion about that doesn't change and I think it's more consistent that all blocks has the same selected style. So I agree with your proposal. I also wonder if working on that isn't a good moment to go back to discussion about the logic of dimming blocks that are not selected , what do you think ? |
Thanks for the info on the current state of the code. It should make these changes pretty straightforward from the sounds of it!
I think it’s worth considering, although dimming is a much more useful indicator for nested blocks because they get more complex than non-nested ones. I’m open to suggestions for dimming/highlighting logic! |
Yeah, applying full border to other components shouldn't be complicated Plan:
Motivation:
Deselect feature also allow user to "preview" how the designed post is look like without any additional indicators. Dimming all unselected block bring some consistency according to how it works on InnerBlock. WDYT ? |
I like the idea of equalizing the paddings/margins between nested blocks and others.
This would bring simplicity into the UX as well as into the code. |
@jbinda responding to your notes above:
I think it's worth a try, especially if we can get to the "ideal" version of the toolbar.
I'm on the fence about dimming. Maybe we should leave dimming for a special "mode" (similar to web's "spotlight mode") so it is an explicit user decision and the behavior is universal based on that toggle? Regardless of how you get to/see this spotlight mode, can I ask to clarify what the logic would be for dimming? My understanding is that we would dim everything "above" (in the nesting hierarchy) or "outside" of the currently-selected block. In other words, if a block that has InnerBlocks is selected, itself and it's children would be fully visible, and everything outside of that would be dimmed. Is that correct?
I'm not 100% sure what you mean here.
I think that's worth a try, seems like it'd be a nice improvement.
Totally agree. My original idea was that tapping outside of the selected parent would essentially escape out of the current selection (this was with dimming — the idea was that the dimming would behave in the same way that a modal backdrop/scrim would, where tapping it dismisses the modal). This would be a nice improvement but I think it can probably be added as an iteration, not a blocker at this time. |
I have read that its already in progress so soon we can start to give it a shot to all blocks
At this moment it works as you described it.
I don't have strong opinion on dimming in way I mentioned. Probably we can see if the current dimming logic is still fine after we apply the solid full border to all blocks.
Great !
Sure we can evaluate it later - small steps is better :) Summarising all above I see three action items at this moment:
Is that correct ? |
Just to clarify, this applies to all selected blocks (not children, which get dashed borders).
Let's try it and see how it feels 😄 |
Ok, will prepare working example and share results |
Please check this out :) |
As discussed in Slack, the borders look great, and the floating toolbar works as expected. I think we can ship this and iterate if necessary. Also, (in a separate ticket) we might want to consider removing the |
Just wanted to add a +1 to removing the "Up" button for top level blocks. |
While working on InnerBlocks, we decided to try a new style for borders to highlight the difference between nested and non-nested blocks. I must say, after playing w/ blocks that use this method, it feels right for all blocks, so I'm proposing that we extend to use this across the board.
Essentially what we did was move the left/right borders in towards the center so that there were slight optical margins on the left/right sides of the block. The two approaches look like this in practice (both are in use at the moment):
Note that all blocks (other than special cases w/ columns, etc) span 100% of the content container, but we used a mix of margin and padding to define the border so that there is an optical inset. So instead of using 12px/16px as the padding on all blocks (and no margin), we use something like 12px/12px, and then there is a 1px border and 3px margins on left/right of the block (don't quote me on these exact values because I can't recall what we're using in the code). This might help to illustrate the difference:
There are a few wins we get by doing this:
// cc @lukewalczak @jbinda @pinarol @hypest — any thoughts?
The text was updated successfully, but these errors were encountered: