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

Render non filterable text facets in a list #4700

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

leenagupte
Copy link
Contributor

@leenagupte leenagupte commented Mar 14, 2025

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

There's a case where a facet could have an array of allowed values but
not be filterable, so a link to a finder shouldn't be rendered.

This was not being picked up so rather than rendering:

Reason for protection: Application to UK scheme from 2021

A hash was rendered instead:

Reason for protection:
{:label=>"Application to UK scheme from 2021", :value=>"uk-gi-after-2021"}

Why

This case was missed when the specialist documents were moved over.

How

A new type is added to account for this case "preset_text". This is
"text" that the publisher selected from a redefined set of options, but
is not filterable so shouldn't be a link.

Screenshots?

On GOV.UK (government-frontend)

Screenshot 2025-03-14 at 10 48 37

Before (frontend)

Screenshot 2025-03-14 at 10 19 22

After

Screenshot 2025-03-14 at 10 43 53

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4700 March 14, 2025 10:50 Inactive
@leenagupte leenagupte force-pushed the render-non-filterable-text-facets branch from b66a4a5 to 3ab472d Compare March 14, 2025 11:03
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4700 March 14, 2025 11:03 Inactive
There's a case where a facet could have an array of allowed values but
not be filterable, so a link to a finder shouldn't be rendered.

This was not being picked up so rather than rendering:

```
Reason for protection: Application to UK scheme from 2021
```

A hash was rendered instead:

```
Reason for protection:
{:label=>"Application to UK scheme from 2021", :value=>"uk-gi-after-2021"}
```

A new type is added to account for this case "preset_text". This is
"text" that the publisher selected from a redefined set of options, but
is not filterable so shouldn't be a link.
This takes the label value with the new "preset_text" type and renders
it as a comma separated list.

The change from using an if/else to a case statement was enforced by
rubocop after the addition of a second "elsif".
@leenagupte leenagupte force-pushed the render-non-filterable-text-facets branch from 3ab472d to 605ea6d Compare March 14, 2025 15:35
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4700 March 14, 2025 15:36 Inactive
@leenagupte leenagupte marked this pull request as ready for review March 14, 2025 15:38
Copy link
Contributor

@deborahchua deborahchua left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@KludgeKML KludgeKML left a comment

Choose a reason for hiding this comment

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

LGTM2!

@leenagupte leenagupte merged commit 7de7180 into main Mar 17, 2025
13 checks passed
@leenagupte leenagupte deleted the render-non-filterable-text-facets branch March 17, 2025 17:38
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