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(ui): improve CoreFileInput #494

Merged
merged 2 commits into from
Aug 13, 2024
Merged

feat(ui): improve CoreFileInput #494

merged 2 commits into from
Aug 13, 2024

Conversation

madeindjs
Copy link
Collaborator

The main issue of CoreFileInput was that, when it re-render, the files selected label is cleared and there is no way to set it again.

To prevent that, the easiest way is to hide the <input type="file" /> and display instead a custom button and a label.

This unlock a cool possibility to create the link of the file and let the user download the file.

Screencast.from.2024-07-18.23-00-18.webm

@madeindjs madeindjs self-assigned this Jul 18, 2024
@ramedina86
Copy link
Collaborator

I like the logic of this one but I think the button needs to be changed.

Can we try using a plainWdsButton? By plain I mean separatorColor as border, then transparent or containerBackgroundColor for the background, primaryTextColor for the text.

From the design system:

Screenshot 2024-07-30 at 13 28 33

The main issue of `CoreFileInput` was that, when it re-render, the files selected label is cleared and there is no way to set it again.

To prevent that, the easiest way is to hide the `<input type="file" />` and display instead a custom button and a label.

This unlock a cool possibility to create the link of the file and let the user download the file.
@madeindjs
Copy link
Collaborator Author

@ramedina86 , do you think we can try to reuse the WdsButton and pass CSS var to customize it ?

It seems a bit hard for now though, the border and background can't be separated for instance

border: 1px solid var(--buttonColor);
background: var(--buttonColor);

@ramedina86
Copy link
Collaborator

@madeindjs please implement a variant prop, we'll need it anyway. If you check WDS we have several variants, so it makes sense to make it user-configurable for other buttons.

@ramedina86
Copy link
Collaborator

Unsure if I was 100% clear, it'd work like

<WdsButton variant="primary">Button that looks like today</WdsButton>
<WdsButton variant="tertiary">Button that's neutral<WdsButton>

@madeindjs
Copy link
Collaborator Author

@ramedina86 , It's done

@ramedina86
Copy link
Collaborator

Nice, looks really good!

@ramedina86 ramedina86 merged commit 8a80db3 into writer:dev Aug 13, 2024
16 checks passed
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