8000 Replace `Choices.js` with `slim-select` by zoglo · Pull Request #8351 · contao/contao · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Replace Choices.js with slim-select #8351

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

Draft
wants to merge 16 commits into
base: 5.5
Choose a base branch
from
Draft

Conversation

zoglo
Copy link
Member
@zoglo zoglo commented May 13, 2025

Description

Since introducing choices to the backend, we have had to modify it a lot due to its nature of wrapping i 8000 tself, triggering a disconnect and then a reconnect in the DOM.

There is nothing against choices, and I consider it the most accessible fit for our case, but circumventing around a "design choice" (Choices-js/Choices#18 (comment)) creates more of a headache than simply replacing it with an easier-to-implement solution.

Hmmm, the beauty of wrapping the original element is that you can query for it from the outer container rather than querying the whole document. Looking for adjacent elements is possible but seems awkward to me - especially as I'm looking for ways to trigger the normal select box on mobile so would need the original element contained.

Workarounds we already tried

After creating the initial PRs (#4363 and #7824) and trying to use it with Hotwired Turbo and Stimulus, we had quite a few fixes, making a usually normal Stimulus controller more complex than needed:

However, as the journey continues, the move to the shadow root already broke the attached onchange listener on, e.g., the content element selection, sending the old value instead of the new one, even tho the change event was the value change from the shadow root.

We don't know what other bugs may occur, but I consider this a sunk-cost fallacy trying to dance around a design decision from 2016 simply because of being a "beauty of wrapping the original element".

Solution

As (not briefly) discussed, it would be easier to simply replace it so here we go.

ToDo's

  • Check for a possibility to add placeholder values
  • Testing with Turbo
  • Remove leftovers from choices-js
  • Adjust the initialization
  • Adjust the styling to match Contao's Backend
  • Dark-Mode styles
  • Add a possibility to modify the configuration

@zoglo zoglo added this to the 5.6 milestone May 13, 2025
@zoglo zoglo requested review from fritzmg and m-vo May 13, 2025 13:19
@zoglo zoglo self-assigned this May 13, 2025
@zoglo zoglo added the feature label May 13, 2025
@fritzmg
Copy link
Contributor
fritzmg commented May 13, 2025

I would do it as a bugfix for 5.5 though - as it's already broken there anyway.

@zoglo
Copy link
Member Author
zoglo commented May 13, 2025

I would do it as a bugfix for 5.5 though - as it's already broken there anyway.

We could do so as there is not yet a possibility to change the configuration but we would need to make sure that libraries using the controller would not break (e. g. having contao--choices as an attribute but using contao--slim-select.

Wdyt about that @leofeyer ?

@leofeyer
Copy link
Member

I lean towards "new feature for Contao 5.6". But the current implementation is also broken enough for a bugfix 😄

@zoglo zoglo force-pushed the feat/slim-select branch from 78c8e55 to 90be883 Compare May 13, 2025 14:18
@zoglo zoglo changed the base branch from 5.x to 5.5 May 13, 2025 14:19
@zoglo zoglo added bug and removed feature labels May 13, 2025
@zoglo zoglo modified the milestones: 5.6, 5.5 May 13, 2025
@zoglo
Copy link
Member Author
zoglo commented May 13, 2025

Styles have been adjusted in f4db717

image
image

Comment on lines 5 to 10
static values = {
options: {
type: Object,
default: {}
}
}
Copy link
Contributor
@fritzmg fritzmg May 13, 2025

Choose a reason for hiding this comment

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

Just wanted to mention: if we don't need any special default settings then we can shorten this to:

Suggested change
static values = {
options: {
type: Object,
default: {}
}
}
static values = {
options: Object
}

In Slack I assumed we'd need some, but may be not? Then Slim Select is even more perfect for us 😁

// though I guess we'll need some settings for translations

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we do. I still have to research the potential configuration options

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

Successfully merging this pull request may close these issues.

3 participants
0