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

Removed highlight of side nav items on search results page [WD-2276] #4682

Merged
merged 1 commit into from
Mar 6, 2023

Conversation

lorumic
Copy link
Contributor

@lorumic lorumic commented Mar 3, 2023

Done

Removed highlight of side nav items on the search results page.

Fixes #4486

QA

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

  • PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
  • Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
    • if CSS class names are not changed it can be bugfix relesase (x.x.X)
    • if CSS class names are changed/added/removed it should be minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.
  • Documentation side navigation should be updated with the relevant labels.

@webteam-app
Copy link

Demo starting at https://vanilla-framework-4682.demos.haus


docs_slug = "" if docs_slug == "/docs" else docs_slug
docs_slug = "" if docs_slug == "/docs" else docs_slug
Copy link
Member

Choose a reason for hiding this comment

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

Why not add the exception here, where we already have a "/docs exception

Not sure if that would work but something like:

docs_slug = "" if docs_slug == "/docs" or docs_slug == "/docs/search" else docs_slug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the first thing that I tried, but actually the /docs/ part of the path gets already replaced one instruction before:

        docs_slug = (
            flask.request.path.replace("/docs/", "")
            .replace("/design/", "")
            .replace("/accessibility", "")
        )

so this condition: docs_slug == "/docs/search" will never be matched.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right!

Copy link
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bartaz bartaz merged commit d91c4d9 into canonical:main Mar 6, 2023
@lorumic lorumic deleted the fix-search-highlight branch March 6, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Search" patterns selected in side nav on search results page
3 participants