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

Navigation Block: vertically align items. #18399

Closed
wants to merge 1 commit into from

Conversation

mtias
Copy link
Member

@mtias mtias commented Nov 8, 2019

This ensures items are properly aligned — depending on the font size the inserter can be misplaced.

Before:
image

After:
image

Might still need some work when selecting an item.

@mtias mtias added the [Block] Navigation Affects the Navigation Block label Nov 8, 2019
@jasmussen
Copy link
Contributor

Nice.

The alignment is better, but there's a shift now on select:

items

@getdave
Copy link
Contributor

getdave commented Nov 11, 2019

@mtias Will you be continuing working on this PR or would you like someone to take it over? Also is it "Blocking"?

@frontdevde
Copy link
Contributor

Just noticed that when navigation items wrap this change has the unintended side effect of aligning everything in the vertical center. Not sure where we'd want the block appender to sit in that case but I assume we'd want the menu items to stay top-aligned.

Before:
Screen Shot 2019-11-12 at 10 34 18

After:
Screen Shot 2019-11-12 at 10 35 10

@frontdevde
Copy link
Contributor

I know it's not an ideal solution but would it be an option to just give the appender button a 1px margin-top? That would nudge it down a little, make it look centered on the single line but wouldn't affect the layout beyond that.

@jasmussen
Copy link
Contributor

We need to be careful to not pixel-optimize towards any given font, as each font will have different needs here. Not doing anything is an option, even if I appreciate the intent of this ticket.

@mtias
Copy link
Member Author

mtias commented Nov 13, 2019

Will you be continuing working on this PR or would you like someone to take it over?

I won't have time soon to finish it. Definitely not blocking, but I think it's worth trying to address as these details compound.

@getdave getdave added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Jan 8, 2020
@apeatling
Copy link
Contributor

I don't see this issue anymore, I think we can close? Tested on a few different themes with editor styles.

Screen Shot 2020-01-15 at 11 37 12 AM

@retrofox
Copy link
Contributor

retrofox commented Feb 4, 2020

It has tackled in #19681. Please feel free to re-open or create a new one if you consider that it's needed.

@retrofox retrofox closed this Feb 4, 2020
@aristath aristath deleted the update/align-navigation-block-items branch November 10, 2020 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block Good First Issue An issue that's suitable for someone looking to contribute for the first time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants