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: refactor dropdown to make it reusable #5675

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

hom3mad3
Copy link
Contributor

@hom3mad3 hom3mad3 commented Jul 22, 2024

fixes #5587, #5596, #5593

@hom3mad3 hom3mad3 changed the base branch from main to dev July 22, 2024 14:37
@hom3mad3 hom3mad3 requested a review from goapunk July 22, 2024 15:01
Copy link
Contributor

@goapunk goapunk left a 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

}

openDropdown () {
this.dropdown.style.display = 'block'
Copy link
Contributor

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 ?

Copy link
Contributor Author

@hom3mad3 hom3mad3 Jul 23, 2024

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

Copy link
Contributor Author

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

@hom3mad3 hom3mad3 force-pushed the mr-2024-07-unify-dropdowns branch 3 times, most recently from 7a7c60b to 504a29d Compare July 23, 2024 13:42
@hom3mad3 hom3mad3 force-pushed the mr-2024-07-unify-dropdowns branch from 504a29d to 523f0a6 Compare July 25, 2024 07:54
@hom3mad3 hom3mad3 requested a review from goapunk July 25, 2024 07:55
@hom3mad3 hom3mad3 changed the title refactor dropdown to make it reusable fix: refactor dropdown to make it reusable Jul 25, 2024
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

LGTM

@goapunk goapunk merged commit 2c68f28 into dev Jul 25, 2024
3 checks passed
@goapunk goapunk deleted the mr-2024-07-unify-dropdowns branch July 25, 2024 08:59
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.

Unify dropdown functionality across all dropdowns
2 participants