-
Notifications
You must be signed in to change notification settings - Fork 6
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: refactor dropdown to make it reusable #5675
Conversation
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.
Works nicely and looks good, just some small remarks/questions
meinberlin/apps/users/templates/meinberlin_users/user_indicator.html
Outdated
Show resolved
Hide resolved
} | ||
|
||
openDropdown () { | ||
this.dropdown.style.display = 'block' |
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 fine with this but just wondering if having it in the css is a bit of a clearer division between functionality and styling as we have the is-open
class to apply it ?
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.
good catch, didn´t pay attention as things were kinda recycled. also added BEM for consistency so now its a modifier dropdown--open
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.
oops i amended with previous commit by mistake
7a7c60b
to
504a29d
Compare
504a29d
to
523f0a6
Compare
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.
LGTM
fixes #5587, #5596, #5593