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(core/menu): adapt width #514

Merged
merged 5 commits into from
May 22, 2023
Merged

fix(core/menu): adapt width #514

merged 5 commits into from
May 22, 2023

Conversation

goncalosard
Copy link
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (yarn build) was run locally and any changes were pushed
  • Unit tests (yarn test) were run locally and passed
  • Visual Regression Tests (yarn visual-regression) were run locally and passed
  • Linting (npm lint) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bug fix
  • Feature
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

[IX-74]

What is the new behavior?

  • Changed width to the new size
  • Adapted basic-navigation, map-navigation, menu-about-news, menu-avatar, menu-item to the changed menu width

Does this introduce a breaking change?

  • Yes
  • No

Testing

Other information

Copy link
Collaborator

@nuke-ellington nuke-ellington left a comment

Choose a reason for hiding this comment

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

The margin values look a little strange to me (e.g. 3.27rem instead of 3.25rem). Is that on purpose?

@goncalosard
Copy link
Contributor Author

The margin values look a little strange to me (e.g. 3.27rem instead of 3.25rem). Is that on purpose?

If its 3.25rem, it will not be perfectly aligned with the ix-menu, meaning some pixels will go under it.

@danielleroux danielleroux changed the title fix(core/menu): new width fix(core/menu): adapt with May 12, 2023
@danielleroux danielleroux changed the title fix(core/menu): adapt with fix(core/menu): adapt width May 12, 2023
Copy link
Collaborator

@danielleroux danielleroux left a comment

Choose a reason for hiding this comment

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

  • Visual issues
  • We should also improve the visual regression tests.

About News

Not aligned correnlty in expand state:
image

Menu Overlay

Gap on the right side:

image

Menu Avatar

Hover is not correct aligned anymore

image

@danielleroux danielleroux added the pull request affects patch version The pull request affects only patch version label May 12, 2023
@danielleroux danielleroux added this to the 1.6.0 milestone May 12, 2023
@danielleroux danielleroux merged commit a7163be into main May 22, 2023
7 checks passed
@danielleroux danielleroux deleted the fix/menu-width branch May 22, 2023 06:13
nuke-ellington pushed a commit that referenced this pull request May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull request affects patch version The pull request affects only patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants