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.

Sign up for GitHub

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

Jump to bottom

make the remove icon of the file preview configurable and default to RemoveCircle #8756

Merged
merged 5 commits into from
Apr 24, 2023
Merged

make the remove icon of the file preview configurable and default to RemoveCircle #8756

merged 5 commits into from
Apr 24, 2023

Conversation

PennyJeans
Copy link

@PennyJeans PennyJeans commented Mar 22, 2023 •
edited by fzaninotto
Loading

Closes #8755

@fzaninotto
Copy link
Member

fzaninotto commented Mar 22, 2023 •
edited
Loading

Thanks for your PR. What's the use case for this?

Edit: Sorry, just saw the attached issue. Looking at it.

@fzaninotto fzaninotto changed the title make the remove icon of the file preview configurable and default to … make the remove icon of the file preview configurable and default to RemoveCircle Mar 22, 2023
@fzaninotto
Copy link
Member

OK, can you please PR against next rather than master as it's a new feature, and update the documentation to mention the new prop?

@fzaninotto
Copy link
Member

Bump, would you mind revisiting this PR?

@PennyJeans PennyJeans changed the base branch from master to next April 12, 2023 06:55
@PennyJeans
Copy link
Author

Thank you, I changed the base to next and updated the docs. Also I refactored a little, so that the default icon is set similarly to the way it is already done in BulkDeleteWithUndoButton .

slax57
slax57 requested changes Apr 12, 2023
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Also, can you add a story to both FileInput.stories.tsx and ImageInput.stories.tsx to demonstrate this new prop? Thanks!

docs/FileInput.md Outdated Show resolved Hide resolved
docs/FileInput.md Outdated Show resolved Hide resolved
docs/ImageInput.md Outdated Show resolved Hide resolved
docs/ImageInput.md Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/FileInputPreview.tsx Outdated Show resolved Hide resolved
@PennyJeans PennyJeans requested a review from slax57 April 14, 2023 08:23
slax57
slax57 requested changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

The stories are working fine 🙂 (except the image preview which I pointed out).

And I'm happy with the React component vs. element solution 🙂

docs/FileInput.md Outdated Show resolved Hide resolved
docs/FileInput.md Outdated Show resolved Hide resolved
docs/FileInput.md Outdated Show resolved Hide resolved
docs/ImageInput.md Outdated Show resolved Hide resolved
docs/ImageInput.md Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/FileInputPreview.tsx Show resolved Hide resolved
packages/ra-ui-materialui/src/input/ImageInput.stories.tsx Outdated Show resolved Hide resolved
@PennyJeans PennyJeans requested a review from slax57 April 18, 2023 09:59
slax57
slax57 approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@slax57 slax57 added the RFR Ready For Review label Apr 24, 2023
@slax57 slax57 added this to the 4.10.0 milestone Apr 24, 2023
@slax57 slax57 merged commit c933ab5 into marmelab:next Apr 24, 2023
@slax57 slax57 mentioned this pull request Apr 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slax57 slax57 slax57 approved these changes

Assignees
No one assigned
Labels
RFR Ready For Review
Projects
None yet
Milestone
4.10.0
Development

Successfully merging this pull request may close these issues.

Make the remove Icon of the FileInputPreview configurable
3 participants
@PennyJeans @fzaninotto @slax57