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

styling of selectize menus #13176

Merged
merged 7 commits into from
May 26, 2023
Merged

Conversation

i-just
Copy link
Contributor

@i-just i-just commented May 10, 2023

Description

  • changed max-height from the default 200px to 70vh
  • kept the options height as is (so it’s still easy to select an option via click/touch)
  • added a horizontal line above optgroup headings (except when it’s the first item on the list)
  • indented options under the optgroup heading so it’s clear that they belong to a group
  • moved selectize initialisation for multi-select to a template and applied the same “show above if there’s not enough space to show below” logic to it

Dropdown (before & after):
Screenshot 2023-05-10 at 10 43 19
Screenshot 2023-05-10 at 10 44 18

Multi-select (before & after):
Screenshot 2023-05-10 at 10 43 38
Screenshot 2023-05-10 at 10 44 25

Related issues

dev-1124

@linear
Copy link

linear bot commented May 10, 2023

@i-just i-just marked this pull request as ready for review May 10, 2023 14:03
@i-just i-just requested a review from a team as a code owner May 10, 2023 14:03
@brandonkelly brandonkelly changed the base branch from develop to 4.5 May 13, 2023 12:28
Copy link
Member

@brandonkelly brandonkelly left a comment

Choose a reason for hiding this comment

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

The multiselect.twig template (and Cp::multiSelectHtml()) should be left alone. We can either:

  • add a new multiselectize.twig template + Cp::multiSelectizeHtml() and Cp::multiSelectizeFieldHtml() methods; or
  • check for a multi variable in the selectize.twig template

Second option is probably easier.

@i-just
Copy link
Contributor Author

i-just commented May 16, 2023

I went a slightly different way regarding those amends. multiselect.twig template is back to what it used to be; Cp::multiSelectHtml() is tweaked slightly, plus I moved the code that determines whether the dropdown shows above or below, so a dropdown and a multi-select can use it.

If you’re not happy with this and you want to strictly avoid any changes to Cp::multiSelectHtml(), I think the only option will be to go with a new multiselectize.twig template + Cp::multiSelectizeHtml() and Cp::multiSelectizeFieldHtml() methods, but happy to discuss this further if you’d like.

@i-just i-just requested a review from brandonkelly May 16, 2023 09:24
@brandonkelly
Copy link
Member

Cp::multiSelectHtml() is tweaked slightly

Did you push everything up? Not seeing any evidence of this in on https://github.com/craftcms/cms/pull/13176/files.

@i-just
Copy link
Contributor Author

i-just commented May 16, 2023

Yes, everything's pushed; I just described it wrong... I was supposed to say: MultiSelect::inputHtml() is tweaked slightly. Sorry!

# Conflicts:
#	src/web/assets/cp/dist/cp.js
#	src/web/assets/cp/dist/cp.js.map
#	src/web/assets/cp/dist/css/cp.css
#	src/web/assets/cp/dist/css/cp.css.map

[ci skip]
[ci skip]
[ci skip]
[ci skip]
@brandonkelly brandonkelly merged commit 263cb4c into 4.5 May 26, 2023
@brandonkelly brandonkelly deleted the feature/dev-1124-selectize-styling branch May 26, 2023 19:00
@brandonkelly
Copy link
Member

Just updated _includes/forms/selectize.twig to support a multi param, and switched all multi-select Selectize inputs over to Cp::selectizeHtml() + multi: true.

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.

2 participants