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/dropdown): breadcrumb dropdown overflow #129

Merged
merged 9 commits into from Nov 17, 2022

Conversation

goncalosard
Copy link
Contributor

Summary

Breadcrumb dropdown now has the max-height for 5 items, after that it will have a scroll for the overflow.

Fixes #77

How did you test this change?

Tested locally

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.

Please add a visual regression testcase

@danielleroux danielleroux added this to the 1.1.0 milestone Nov 11, 2022
@danielleroux danielleroux added the pull request affects patch version The pull request affects only patch version label Nov 11, 2022
@danielleroux danielleroux modified the milestones: 1.1.0, 1.1.1 Nov 11, 2022
@goncalosard
Copy link
Contributor Author

Please add a visual regression testcase

@danielleroux The fix is done and the visual regression test is done. Its not passing the build only here, on my machine works perfect. I think its because it is expecting to have a snapshot from a linux, you should just run the tests, and commit the snapshot.

@danielleroux
Copy link
Collaborator

Please add a visual regression testcase

@danielleroux The fix is done and the visual regression test is done. Its not passing the build only here, on my machine works perfect. I think its because it is expecting to have a snapshot from a linux, you should just run the tests, and commit the snapshot.

Did you executed the test inside Docker?

I will update the snapshots.

@danielleroux
Copy link
Collaborator

@goncalosard I checked the PR i will refactor the changes regarding the breadcrumb a little bit, because it is more sustainable to adapt the max-height within the dropdown it self

@danielleroux danielleroux removed their assignment Nov 17, 2022
@danielleroux danielleroux modified the milestones: 1.1.1, 1.1.0 Nov 17, 2022
@danielleroux
Copy link
Collaborator

@nuke-ellington @goncalosard Can you please review the changes, i have made regarding dropdown?

I will also resolve the merge conflicts

@danielleroux danielleroux changed the title fix(core): breadcrumb dropdown overflow fix(core/dropdown): breadcrumb dropdown overflow Nov 17, 2022
@danielleroux danielleroux merged commit d801ddd into main Nov 17, 2022
@danielleroux danielleroux deleted the fix/breadcrumb-dropdown branch November 17, 2022 12:42
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
Status: Done
Development

Successfully merging this pull request may close these issues.

Breadcrumb dropdown does not have max height
3 participants