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

WIP: Rewrite LiveSearch as an ES Module #5370

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kr8n3r
Copy link
Contributor

@kr8n3r kr8n3r commented Feb 6, 2025

We want to migrate all of our legacy Javascript files to ESM, so this is the latest in the series
of conversions.

Rewrite follows the same conventions we previously used for FocusBanner, Collapsible checkboxes,
ColourPreview, FileUpload, Autofocus, PreviewPane and CopyToClipboard.

Added bonus: removes the use of jQuery.

In this PR we:

  • rewrite the module as ES Module
  • update the tests to be ES Module
  • remove old module JS file
  • remove old module from the legacy bundle build

How to test

  • templates page search
  • team members search
  • change branding search
  • /organisations//settings/email-branding/add
  • /organisations//settings/letter-branding/add
  • /organisations//settings/add-nhs-local-organisation for NHS services
  • /services//service-settings/link-service-to-organisation
  • /services/>id>/service-settings/(set-email/letter)-branding

@kr8n3r kr8n3r force-pushed the rewrite-livesearch-as-esm branch from 7dd9ab3 to ed0bb95 Compare February 6, 2025 13:48
@kr8n3r kr8n3r force-pushed the rewrite-livesearch-as-esm branch from ed0bb95 to b2bd12f Compare February 6, 2025 13:51
@kr8n3r kr8n3r marked this pull request as ready for review February 6, 2025 13:51
Copy link
Contributor

@tombye tombye left a comment

Choose a reason for hiding this comment

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

This looks good to me. I spotted a few things, one a bit pedantic and the other more of a question. Everything else great though 👍🏻

app/assets/javascripts/esm/live-search.mjs Outdated Show resolved Hide resolved
app/assets/javascripts/esm/live-search.mjs Outdated Show resolved Hide resolved
We want to migrate all of our legacy Javascript
files to ESM, so this is the latest in the series
of conversions.

Rewrite follows the same conventions we previously
used for FocusBanner, Collapsible checkboxes,
ColourPreview, FileUpload, Autofocus,
PreviewPane and CopyToClipboard.

Added bonus: removes the use of jQuery as well
@kr8n3r kr8n3r force-pushed the rewrite-livesearch-as-esm branch from 4b165ad to c036ea6 Compare February 7, 2025 13:10
@kr8n3r kr8n3r changed the title Rewrite LiveSearch as an ES Module WIP: Rewrite LiveSearch as an ES Module Feb 11, 2025
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Feb 11, 2025

spotted an issue with template search :(

We use hidden attribute to hide items that don't match live-search results.

Normally, the broser would hide the element, but as these ones have the display property set, we need to set additional override styles.
There are some items that are hidden by default with CSS and which we need to show if they match the search query.

Whiel we do the visibility toggling by applying the `hidden` attribute to the each item, we cannot use the same attribute of the hiden-by-default items, as the checkbox macro from DS does not accomodate for that, hence they're hidden by css class.

This adds a bit more logic to the JS to apply a new class to those items when they match the search query and use CSS to show them, as well as remove the same class when they don't match or the search query is empty (default state)
When they match the search query
@kr8n3r kr8n3r force-pushed the rewrite-livesearch-as-esm branch from c036ea6 to 8966f33 Compare February 14, 2025 16:17
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