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

Fix: Navigation item toolbar buttons unaligned #18747

Merged

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Nov 26, 2019

Description

Regressed in #18631

The PR #18631 removed some styles that caused a bug on the navigation block toolbar. This PR readds the relevant styles to avoid this bug.

How has this been tested?

I added a navigation block and I verified the item toolbar looked as expected.

Screenshots

After:
image

Before:
image

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block labels Nov 26, 2019
@youknowriad
Copy link
Contributor

I think ideally we don't need these styles and the "buttons" should have a consistent height across the UI. in toolbars or not.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving anyway as it's not big harm but thought I'd explain the reasoning.

@jorgefilipecosta jorgefilipecosta merged commit d78fe98 into master Nov 26, 2019
@jorgefilipecosta jorgefilipecosta deleted the fix/navigation-item-toolbar-buttons-unaligned branch November 26, 2019 12:37
@jorgefilipecosta
Copy link
Member Author

I think ideally we don't need these styles and the "buttons" should have a consistent height across the UI. in toolbars or not.

Should we move the height styles to the button component?

@youknowriad
Copy link
Contributor

Should we move the height styles to the button component?

yes, maybe. It will probably have impacts, so we need to do this carefully. We can also remove the width of the toolbar buttons, I can see large toolbar buttons with text...

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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants