Skip to content

Adopt the data-ps-* convention across the theme - #1096

Draft
tblivet wants to merge 16 commits into
PrestaShop:2.xfrom
tblivet:feat/data-ps-migration
Draft

Adopt the data-ps-* convention across the theme#1096
tblivet wants to merge 16 commits into
PrestaShop:2.xfrom
tblivet:feat/data-ps-migration

Conversation

@tblivet

@tblivet tblivet commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
Questions Answers
Description? Adopts the data-ps-* convention from #934 across the theme. The selector map goes from 42% to 96% data-ps-*, and the legacy js-* hooks bound by the theme's JS drop from 65 to 0. Remaining non-data-ps entries are commented in place with the reason they stay (Bootstrap classes its own JS resolves, and two data-link-action values).
Type? refactor
BC breaks? 6 dead keys removed from Theme.selectors: backgroundCircle, idProductInput, range, searchFilterControls, searchFiltersWrapper, searchFilterToggler. No class or id removed from the markup.
Deprecations? The js-* classes left in the markup are no longer read by any theme JS and are kept for retro-compatibility only removal should be planned for v3.0
Fixed ticket? Related to this discussion #934
Sponsor company @PrestaShopCorp
How to test? --

@ps-jarvis

Copy link
Copy Markdown

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

  • Shop.Theme.Catalog
    • Remove %1$s filter: %2$s
    • Clear search
    • No products available at the moment
    • Products details
  • Shop.Theme.Global
    • Product filters
    • Change language
    • Close search
  • Shop.Theme.Actions
    • Go to previous page
    • Go to next page
    • Apply voucher
    • Back to %step_title%
    • Edit your shipping method
    • Edit addresses
  • Shop.Theme.Menu ⚠️
    • Go back to main menu
  • Shop.Theme.Checkout
    • Gift(s)
    • Products in cart
    • Terms and conditions
    • Your cart could not be updated. Please try again.

(Note: this is an automated message, but answering it will reach a real human)

<div class="modal-body">
<div class="row">
<div class="col-md-12 col-sm-12" id="{$modal_id}-message">
<div class="col-md-12 col-sm-12" id="{$modal_id}-message" data-ps-ref="alert-modal-message">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

col-md-12 col-sm-12 + row can go :-)

@Hlavtox

Hlavtox commented Sep 7, 2026

Copy link
Copy Markdown
Member

Thanks @tblivet, this is the biggest pain when working with the themes.

Does it also help with the targetting in general? (A client wants to move a voucher block somewhere in the cart - refresh broken. A client wants to move some things around on product page - refresh broken.)

@tblivet

tblivet commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Hlavtox!

I share your point of view. The objective is to put this convention in place so we can rely on meaningful data attributes and clearly identify the JS responsibility of an element, instead of relying on js-* classes or even random selectors in some cases.

For now the attributes are added alongside the old classes, which stay in the markup for retro-compatibility even though the theme's JS no longer reads them. That way nothing breaks. But to make it complete we should get rid of the old selectors, and also tackle themes/_core/js on the core side to give us more customization capabilities, though some template dependencies will remain, due to how core works and returns templates.

And to answer your question, moving a block should keep working as long as it carries its attribute, since the JS will find it wherever it is placed. What will stay fragile is anything core replaces as a whole rendered chunk moving a block out of that container is not something the theme can fix on its own.

@Hlavtox

Hlavtox commented Sep 8, 2026

Copy link
Copy Markdown
Member

@tblivet Just FYI, there is an option in the core that allows theme.yml to disable core.js loading. We implemented it because of hummingbird. The vision was to drop usage of core.js and implement these things custom way in hummingbird.

@tblivet

tblivet commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the info @Hlavtox, I didn't know this had been introduced for Hummingbird!

Yes, that's something we'll have to decide for a v3 / v10 i think. Dropping core.js is what would let us really remove the remaining js-* hooks instead of just not reading them anymore. Before that we need to check the impact on third party modules that rely on prestashop.selectors and on core events. Worth its own discussion, I'll keep it in mind for the next major. That's a big subject!

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.

4 participants