Skip to content

feat(lxl-web): Remove redundant search params (LWS-405)#1352

Open
johanbissemattsson wants to merge 4 commits intodevelopfrom
feat-lxl-web-remove-redundant-search-params
Open

feat(lxl-web): Remove redundant search params (LWS-405)#1352
johanbissemattsson wants to merge 4 commits intodevelopfrom
feat-lxl-web-remove-redundant-search-params

Conversation

@johanbissemattsson
Copy link
Copy Markdown
Contributor

@johanbissemattsson johanbissemattsson commented Aug 18, 2025

Description

Tickets involved

LWS-405

Solves

Removes redundant search params as _limit, _offset, _sort and _spell doesn't seem to be required any longer (the links in the filter panel omits them).

Summary of changes

  • Remove redundant search params
  • Remove redundant search params from tests
  • Delete _limit search param on new searches (these would otherwise be kept after using the pagination)

Comment thread lxl-web/src/lib/components/find/SearchResultSort.svelte Outdated
// Always reset these params on new search
p.set('_offset', '0');
p.delete('_offset');
p.delete('_limit');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dunno if being able to manually change search items per page (using _limit), is a useful hidden feature? That will maybe become a ui feature someday. This prevents that.

I don't have a strong opinion myself...

Copy link
Copy Markdown
Contributor

@jesperengstrom jesperengstrom Aug 18, 2025

Choose a reason for hiding this comment

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

But hmm, pagination links keeps any existing _limit, but facet links throws it away. Wonder if the latter is a bug? @kwahlin

@jesperengstrom jesperengstrom self-requested a review August 19, 2025 08:45
Copy link
Copy Markdown
Contributor

@jesperengstrom jesperengstrom left a comment

Choose a reason for hiding this comment

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

LGTM!

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