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

DDFHER-76 - Add "location" and "sublocation" URL parameters in advanced search (CQL) #1440

Conversation

kasperbirch1
Copy link
Contributor

@kasperbirch1 kasperbirch1 commented Sep 18, 2024

Link to issue

https://reload.atlassian.net/browse/DDFHER-76

Depend on

danskernesdigitalebibliotek/dpl-design-system#735
danskernesdigitalebibliotek/dpl-cms#1585

Description

This pull request implements URL parameters for location and sublocation. Additionally, the parameters from the URL are processed in useEffectOnce, ensuring they are included on the first load. The removeQueryParametersFromUrl function is used to remove the parameters if their strings are empty.

It also refactors the logic to allow users to search across multiple locations or sublocations by using a comma-separated string in the input fields.

Test

http://varnish.pr-1585.dpl-cms.dplplat01.dpl.reload.dk/advanced-search

Skaermoptagelse.2024-09-18.kl.15.49.16.mov

@kasperbirch1 kasperbirch1 changed the title Add "location" and "sublocation" URL parameters in advanced search (CQL) DDFHER-76 - Add "location" and "sublocation" URL parameters in advanced search (CQL) Sep 18, 2024
@kasperbirch1 kasperbirch1 force-pushed the DDFHER-76-filtering-on-onshelf-location-and-sublocation-should-be-reflected-in-links branch from 53f4fb7 to 7173d08 Compare September 18, 2024 13:34
Implements URL parameters for `location` and `sublocation`. Additionally, the parameters from the URL are processed in `useEffectOnce`, ensuring they are included on the first load. The `removeQueryParametersFromUrl` function is used to remove the parameters if their strings are empty.

Currently, the code does not handle multiple "locations" or "sublocations." It is unclear whether this is necessary, and I am awaiting a response from DDF regarding this.
…es in advanced search (CQL)

This allow users to search across several locations or sublocations by using a comma-separated string in the input fields.
- Location
- Sublocation
@kasperbirch1 kasperbirch1 force-pushed the DDFHER-76-filtering-on-onshelf-location-and-sublocation-should-be-reflected-in-links branch from 7173d08 to 230ca57 Compare September 30, 2024 13:37
…` + Add form wrapper

The `Textarea` is added to keep the structure consistent, and the class `advanced-search-cql-form` is used to maintain spacing between elements.
Copy link
Contributor

@kasperg kasperg left a comment

Choose a reason for hiding this comment

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

👍 This seems to work well.

I am not totally satisfied about the code structure in advanced search in the way we handle url parameters etc. but that is none of your fault. The implementation follows what is.

I have a few comments regarding the code structure of the next form components.

I appreciate the updated design for the form elements. I think the form looks better.
Consider if they take up too much vertical space between the title and the form.

Monosnap | DPL CMS 2024-10-01 10-53-36

As I see it some of the issue is caused by different approaches in the "normal" form and in the CQL form. One uses margins. The other grip/gap.

src/apps/advanced-search/CqlSearchHeader.tsx Outdated Show resolved Hide resolved
src/components/forms/label/Label.tsx Outdated Show resolved Hide resolved
src/components/forms/textarea/Textarea.tsx Outdated Show resolved Hide resolved
@kasperg kasperg assigned kasperbirch1 and unassigned kasperg and spaceo Oct 1, 2024
To keep all form elements in the same place, as we do in the design system.
Also update `Label` to add "*" after label name for `required` fields
The 'name' attribute was removed from the <textarea> because its data does not need to be sent to the server during form submission.
…der`

This adjusts the spacing between the title and the form, reflecting the markup in the DPL design system.
This was a leftover from when I moved `TextInput` to the `forms` folder. However, upon review, I noticed that this issue occurred in many places.
@@ -51,7 +45,11 @@ const App = ({ story }) => <Store>{WrappedStory(story)}</Store>;
// Consideration for the future - using addon-redux could bring value.
// It wasn't implemented to begin with because it wasn't compatible with Storybook 6.
export const decorators = [
Story => <><App story={Story} /></>
(Story) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a Prettier (format on save) change. However, it should be the same result.

@kasperg kasperg assigned kasperbirch1 and unassigned kasperbirch1 Oct 1, 2024
@kasperbirch1 kasperbirch1 merged commit dd03394 into develop Oct 1, 2024
20 checks passed
@kasperbirch1 kasperbirch1 deleted the DDFHER-76-filtering-on-onshelf-location-and-sublocation-should-be-reflected-in-links branch October 1, 2024 11:37
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.

3 participants