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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: contextual menu css position #4915
base: main
Are you sure you want to change the base?
feat: contextual menu css position #4915
Conversation
e1c2dd3
to
b4378c9
Compare
@@ -16,7 +16,7 @@ | |||
@extend %vf-has-box-shadow; | |||
|
|||
display: none; | |||
margin: 0; | |||
margin: 0 0 $input-margin-bottom 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is margin on the dropdown related to placement?
Contextual menu is absolutely positioned, so I'm not sure if bottom margin on it affects anything?
@@ -83,6 +83,8 @@ | |||
} | |||
|
|||
.p-contextual-menu__toggle { | |||
// Remove margin bottom for the dropdown menu to be aligned with the toggle | |||
margin-bottom: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we can do that. In our example p-contextual-menu__toggle
is used on the button, and in such case it would override the default margin on the button.
The fact that button has contextual menu should not affect its margin.
I guess this was the main reason to use JS for positioning - so we can calculate top position regardless of margin that element having contextual menu may have.
Done
This PR removes the need for using JavaScript for vertical positioning of the contextual menu and will subsequently allow to simplify implementations of this pattern.
QA
Check if PR is ready for release
If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:
Feature 馃巵
,Breaking Change 馃挘
,Bug 馃悰
,Documentation 馃摑
,Maintenance 馃敤
.package.json
should be updated relative to the most recent release, following semver convention:Screenshots
No visual changes, looks exactly the same way as before. E.g.:
Notes
Somewhat related PR in react-components: canonical/react-components#1000