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

Clear button in searchInput #250

Closed
wants to merge 1 commit into from

Conversation

Abdelra6
Copy link

@Abdelra6 Abdelra6 commented Feb 6, 2024

closes #236

replicate of #240 closed it by accident because of some conflicts

@Abdelra6
Copy link
Author

Abdelra6 commented Feb 6, 2024

@garrett the width of the input is fixed now. I changed the components. I hope that's a better one)

@garrett
Copy link
Member

garrett commented Feb 7, 2024

Oh, we usually re-use the same PR for the same branch so the conversation isn't fragmented and gets lost. That's done with a force push, such as git push --force-with-lease and the branch info (this version checks to make sure your local branch is up to date with the server).

Two problems:

  1. The X icon is always there now, even when nothing was typed.
  2. When clicking the X to clear the entry, I get an "Ooops!" error.

image

My dev console says:

Uncaught TypeError: setCurrentFilter is not a function
    onClick header.jsx:62
    React 15
        callCallback2
        invokeGuardedCallbackDev
        invokeGuardedCallback
        invokeGuardedCallbackAndCatchFirstError
        executeDispatch
        processDispatchQueueItemsInOrder
        processDispatchQueue
        dispatchEventsForPlugins
        dispatchEventForPluginEventSystem
        batchedUpdates$1
        batchedUpdates
        dispatchEventForPluginEventSystem
        dispatchEventWithEnableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay
        dispatchEvent
        dispatchDiscreteEvent

Copy link
Member

@garrett garrett left a comment

Choose a reason for hiding this comment

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

(See above; I forgot to make it a review instead of a normal comment.)

@Abdelra6
Copy link
Author

Abdelra6 commented Feb 7, 2024

Sorry I can't test it completely due to the fsinfo commit. How can I make the [x] button appear without changing the width of the input?

/>
<TextInputGroupUtilities>
<Button
variant="plain" onClick={_ => setCurrentFilter("")}
Copy link
Member

Choose a reason for hiding this comment

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

_ is already bound to cockpit.gettext. Just pass setCurrentFilter("").

icon={<SearchIcon />} value={currentFilter}
onChange={onFilterChange} placeholder={_("Filter directory")}
/>
<TextInputGroupUtilities>
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be behind some conditional like currentFilter !== ""

Copy link
Author

@Abdelra6 Abdelra6 Feb 8, 2024

Choose a reason for hiding this comment

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

Yes but that also will increase the width

@jelly
Copy link
Member

jelly commented May 22, 2024

Superseeded by #465

@jelly jelly closed this May 22, 2024
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.

When filtering returns no results show that to the user
3 participants