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

Optiongroup children should be indented #1474

Closed
mrleblanc101 opened this issue Aug 16, 2021 · 11 comments · Fixed by #1567 · May be fixed by #1568
Closed

Optiongroup children should be indented #1474

mrleblanc101 opened this issue Aug 16, 2021 · 11 comments · Fixed by #1567 · May be fixed by #1568

Comments

@mrleblanc101
Copy link

mrleblanc101 commented Aug 16, 2021

Currently, it's difficult to understand the relation between optiongroup children and their parent as the is no indentation.
The parent has a light grey background, but it's that it's a parent-children relationship.

Capture d’écran, le 2021-08-16 à 15 01 38

I think the optiongroup children should be indented just like in regular select.
Capture d’écran, le 2021-08-16 à 14 47 58

I could do this with custom style, but optiongroup children are not nested inside their parent in the HTML and the children have no specific class, only the optiongroup parent has a class.

If I add padding to all element, then remove the padding for the optiongroup parent, this will affect my other multiselect that do not have optiongroup. There is no global class on the component to know if it has optiongroup or not.

.multiselect__option {
    padding-left: 24px; // Add padding to every element
}
.multiselect__option--group {
    padding-left: 12px; // Remove padding to every parent
}

What it should be:

.multiselect__option--children {
    padding-left: 24px; // Add padding only to children
}

or

.multiselect.has-option-group {
    .multiselect__option {
    padding-left: 24px; // Add padding to every element
    }
    .multiselect__option--group {
    padding-left: 12px; // Remove padding to every parent
    }
}

Capture d’écran, le 2021-08-16 à 14 58 39

@mrleblanc101
Copy link
Author

mrleblanc101 commented Aug 16, 2021

I guess I could use attribute selector but that doesn't seem right either:

.multiselect[group-values],
.multiselect[group-label],
.multiselect[group-select] {
    //My custom CSS
}

EDIT: Nvm, these attr shouldn't even be rendered in the DOM in the first place, I created a wrapper component and forgot to set inheritAttrs: false;

@mrleblanc101
Copy link
Author

I've added the class in my wrapper component using the code bellow.
I still think this could be improved.

:class="{ 'has-option-group': $attrs['group-values'] || $attrs['group-label'] || $attrs['group-select'] }"

@christianbayer
Copy link

I've indented the options though this styling rule:

.multiselect__option:not(.multiselect__option--group) {
  padding-left: 20px;
}

To ensure that only the selectors with groups would be indented, I've also added a class called has-option-groups on the vue-multiselect element, and changed the rule to:

.has-option-groups .multiselect__option:not(.multiselect__option--group) {
  padding-left: 20px;
}

@mrleblanc101
Copy link
Author

@shentao Can I open a PR for this or is there a reason why the option inside an optiongroup wouldn't be indented ?

@shentao
Copy link
Owner

shentao commented Oct 24, 2022

Adding the indentation (as much as I like it) would impact existing styling. I would personally recommend the solution suggested above with a custom CSS rule.

If it could be added as an optional change then I think it’s okay. But I would refer to @mattelen @akki-jat if they would like to merge that.

@mrleblanc101
Copy link
Author

mrleblanc101 commented Oct 24, 2022

@shentao Thanks for your reply. Any change will have impact, it might need to be release in a minor/major instead of a patch release as to not affect existing user.

The main problem with the custom CSS is for other library using depending on vue-multiselect. For exemple, I use outl1ne/nova-multiselect-field so it make it harder to override the styles.

I could contribute a PR to that package or override the style in all my Laravel Nova project, but the best would be to fix it in Vue-multiselect

@akki-jat
Copy link
Collaborator

@mrleblanc101 I would suggest adding proper classes so if anyone wants to overwrite the styles for the group parent and the children can do that and default styles will be the same as those are now

@mrleblanc101
Copy link
Author

mrleblanc101 commented Oct 24, 2022

If you're ok with that I can open a PR, although I strongly feel that vue-multiselect should adopt indentation to match the HTML Select

@akki-jat
Copy link
Collaborator

@mrleblanc101 We can decide the one default style right, in the end we need to provide the capability to change it as the user wants. So for now we can keep it as it is but yes in the next major version we can try to make an indented style default one as I think it is better UX.

@mrleblanc101
Copy link
Author

Do you want me to open 2 PR.
One for the class and one for the default indented style ?

@mattelen
Copy link
Collaborator

@mrleblanc101 I think create one PR for the new class, and for the new default but the PR for the new default might take some time to merge in, as this would be something we can look at changing in V4.

In the meantime we want to release another update of V2, and get the Vue 3 compatible version of V3 published as stable.

TIA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants