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

3060: KebabMenu - Aligned CloseButton based on Position of Expand Button #3072

Merged
merged 4 commits into from
Feb 11, 2025

Conversation

hannaseithe
Copy link
Contributor

@hannaseithe hannaseithe commented Feb 5, 2025

Short description

I aligned the close button with the expand button and changed the height of the header, so that now the KebapMenu Header has the same height as the main header

Proposed changes

  • I have added the container ActionBar (analoguos to the Header structure in Header.tsx)
  • changed height to min-height,so that now the header of the KebapMenu has the same height as the main header
  • adjusted the paddings

Side effects

  • none that I am aware of

Testing

  • Is the close button correctly positioned under all circumstances?

Resolved issues

Fixes: #3060

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Amazing, thank you for your first PR with us! It looks pretty good already, except I think you positioned the closing icon on a page without a scroll bar. When I open it on a page with scrollbar, the closing icon is still a little bit off from where the kebob icon was.

Screen.Recording.2025-02-05.at.15.48.51.mov

web/src/components/KebabMenu.tsx Outdated Show resolved Hide resolved
Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

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

Congrats and thanks a lot for your first contribution 🎉 😍 Nice work, works and looks as expected! Tested on firefox :)

💭 Unrelated to your PR and does not need to be done here: I think the hit targets are quite small, perhaps we should improve add some padding around the icons to increase the button size a bit?

@steffenkleinle
Copy link
Member

Amazing, thank you for your first PR with us! It looks pretty good already, except I think you positioned the closing icon on a page without a scroll bar. When I open it on a page with scrollbar, the closing icon is still a little bit off from where the kebob icon was.
Screen.Recording.2025-02-05.at.15.48.51.mov

I am also not able to reproduce this neither on firefox desktop nor mobile (or if so, only negligible).

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Then I think we're good to go :)

@LeandraH
Copy link
Contributor

You can now click "Update branch" in this PR, and then enable auto-merge 🎉 Or alternatively, you can click "Update branch", wait for that to be finished, and then click to merge the PR.

@hannaseithe hannaseithe merged commit f57906b into main Feb 11, 2025
7 checks passed
@hannaseithe hannaseithe deleted the 3060--align-close-button-of-kebap-menu branch February 11, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix placement of kebab side menu close icon
3 participants