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

OBPIH-5091 Make selected options appear at the top of dropdown after … #3683

Merged
merged 2 commits into from
Dec 6, 2022

Conversation

kchelstowski
Copy link
Collaborator

…closing and opening the dropdown

@@ -129,6 +130,30 @@ class Select extends Component {
}
}

sortOptionsByChecked(options, checkedValues) {
const groupedByChecked =
options?.length ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember a while back I found a nice way for parting array based on a condition just like you have and used this logic.

let checked = [];
let unchecked = [];

options?.forEach((val) => {
    if (checkedValues?.some(val => val.id === curr.id) {
        checked.push(val)
   } else {
       unchecked.push(val)
   }
})

This in my opinion feels shorter and more explicit/easier to read, but if you still would like to stick to reduce then I don;t have a problem maybe just change condition checking for options?.length from a ternary operator ? ... : ... to a simpler if statemenet becasue ternary operator makes the code a little too clutered

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as you probably know, I'm a fan of functional programming and immutable approach, so that's why I prefer the reduce approach instead of forEach function which is a void function.
If it doesn't bother you that much, I'd love to stay with the reduce version, as for if, I can agree and will change it :)

@awalkowiak awalkowiak merged commit 303aa7d into develop Dec 6, 2022
@awalkowiak awalkowiak deleted the OBPIH-5091 branch December 6, 2022 22:19
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.

None yet

3 participants