Skip to content

fix(command-palette): highlighting after search mode#1672

Merged
d-rita merged 5 commits intofeature-command-palettefrom
fix/highlight-after-search
Feb 25, 2025
Merged

fix(command-palette): highlighting after search mode#1672
d-rita merged 5 commits intofeature-command-palettefrom
fix/highlight-after-search

Conversation

@d-rita
Copy link
Copy Markdown
Contributor

@d-rita d-rita commented Feb 21, 2025

Fixes LIBS-756


Description

This PR fixes some highlighting issues after the search filter has been cleared.

  1. It highlights first category item in the category view instead of the back action
  2. It highlights the first item in the default section in the home view

Checklist

  • API docs are generated
  • Tests were added
  • Storybook demos were added

Screenshots

Before:

list.view.-.before.mov

After:

after.-.list.view.mov

@d-rita d-rita requested a review from a team as a code owner February 21, 2025 14:18
@dhis2-bot
Copy link
Copy Markdown
Contributor

dhis2-bot commented Feb 21, 2025

🚀 Deployed on https://pr-1672--dhis2-ui.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify February 21, 2025 14:22 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 21, 2025 14:43 Inactive
@d-rita d-rita requested a review from a team February 24, 2025 10:08
@d-rita d-rita force-pushed the LIBS-753/selectable-back-action branch from 3762e33 to de53c30 Compare February 24, 2025 18:11
Base automatically changed from LIBS-753/selectable-back-action to feature-command-palette February 24, 2025 18:48
@d-rita d-rita force-pushed the fix/highlight-after-search branch from ca9c5e1 to ea7a9f9 Compare February 24, 2025 20:27
@dhis2-bot dhis2-bot temporarily deployed to netlify February 24, 2025 20:31 Inactive
Copy link
Copy Markdown
Contributor

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

LGTM! Left one minor comment

Comment on lines 27 to 35
const activeItems = useMemo(() => {
if (currentView === HOME_VIEW) {
return activeSection === ACTIONS_SECTION ? actionsArray : itemsArray
return filter || activeSection === GRID_SECTION
? itemsArray
: actionsArray
} else {
return filter ? itemsArray : actionsArray.concat(itemsArray)
}
}, [filter, itemsArray, actionsArray, currentView, activeSection])
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.

(minor) I think this logic could be cleaned up a bit so it's easier to read. For example, instead of having multiple ternary operators with the filter condition, it should be possible to just have a single if statement at the beginning. And then the remaining ternary could just be a nested if to make it a little easier to read. Something like this, maybe:

    const activeItems = useMemo(() => {
        if (filter) {
            return itemsArray;
        }
        if (currentView === HOME_VIEW) {
            if (activeSection === GRID_SECTION) {
                return itemsArray
            } else {
                return actionsArray
            }
        } else {
            return actionsArray.concat(itemsArray)
        }
    }, [filter, itemsArray, actionsArray, currentView, activeSection])

Another option would be to combine all branches which yield itemsArray in a single if statement:

    const activeItems = useMemo(() => {
        if (filter || (currentView === HOME_VIEW && activeSection === GRID_SECTION)) {
            return itemsArray;
        }
        if (currentView === HOME_VIEW) {
            return actionsArray
        } else {
            return actionsArray.concat(itemsArray)
        }
    }, [filter, itemsArray, actionsArray, currentView, activeSection])

There are likely other ways to make this a bit more readable, especially by thinking about which variables are needed and how they're named, but regardless it will help to clean up the logic in this memoized callback a bit.

@sonarqubecloud
Copy link
Copy Markdown

@dhis2-bot dhis2-bot temporarily deployed to netlify February 25, 2025 11:39 Inactive
@d-rita d-rita merged commit c7866d6 into feature-command-palette Feb 25, 2025
@d-rita d-rita deleted the fix/highlight-after-search branch February 25, 2025 11:51
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