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

feat: file input #7238

Merged
merged 2 commits into from
May 22, 2024
Merged

feat: file input #7238

merged 2 commits into from
May 22, 2024

Conversation

deboer-tim
Copy link
Collaborator

What does this PR do?

We have Inputs next to Browse buttons in many places, all with slight variations in UI or browsing. This creates a new FileInput component that uses a regular Input component with a folder icon on the right to browse for files. Exposes OpenDialogOptions to make it easy to customize what the file dialog should filter for.

Uses the new FileInput for file/folder preferences so that there's something to test. There are several other file/browse buttons elsewhere, but they would be migrated separately. Likely also a candidate for svelte-ui package, but that should be after we become comfortable with/use it.

Screenshot / video of UI

Before:

Screenshot 2024-05-16 at 11 57 45 AM

After:

Screenshot 2024-05-16 at 11 57 11 AM

What issues does this PR fix or reference?

Fixes #7197.

How to test this PR?

Tests added, check Settings > Preferences for usability/regression.

  • Tests are covering the bug fix or the new feature

We have Inputs next to Browse buttons in many places, all with slight
variations in UI or browsing. This creates a new FileInput component that
uses a regular Input component with a folder icon on the right to browse
for files. Exposes OpenDialogOptions to make it easy to customize what
the file dialog should filter for.

Uses the new FileInput for file/folder preferences so that there's something
to test. There are several other file/browse buttons elsewhere, but they
would be migrated separately. Likely also a candidate for svelte-ui package,
but that should be after we become comfortable with/use it.

Fixes containers#7197.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim deboer-tim requested review from benoitf and a team as code owners May 16, 2024 16:02
@deboer-tim deboer-tim requested review from dgolovin and axel7083 and removed request for a team May 16, 2024 16:02
We have Inputs next to Browse buttons in many places, all with slight
variations in UI or browsing. This creates a new FileInput component that
uses a regular Input component with a folder icon on the right to browse
for files. Exposes OpenDialogOptions to make it easy to customize what
the file dialog should filter for.

Uses the new FileInput for file/folder preferences so that there's something
to test. There are several other file/browse buttons elsewhere, but they
would be migrated separately. Likely also a candidate for svelte-ui package,
but that should be after we become comfortable with/use it.

Fixes containers#7197.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

Just tried and i'm facing some weird behavior. When the field is empty, sometimes it just clear itself, or if i click on reset it does nothing.

inconsisten_behavior

Copy link
Contributor

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Tested, was able to browse it fine and saw the button changes didn't negatively impact any of the other UI components.

Implementation and code LGTM!

@deboer-tim
Copy link
Collaborator Author

Just tried and i'm facing some weird behavior. When the field is empty, sometimes it just clear itself, or if i click on reset it does nothing.

@lstocchi that is unrelated and unfortunately affects all configuration even prior to this. I opened #7142 to track it and you can see it affect other settings.

@feloy
Copy link
Contributor

feloy commented May 22, 2024

Is it possible to clear a value once you have already chosen a directory/file?

I cannot find a way to do this before/after this PR, but as we are creating a specific component, it would be interesting to support this possibility to reset a value to /undefined/

@deboer-tim deboer-tim merged commit aa93256 into containers:main May 22, 2024
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.11.0 milestone May 22, 2024
@deboer-tim deboer-tim deleted the input-file branch May 22, 2024 15:04
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.

FileInput component
6 participants