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

feature: Import action #9364

Merged
merged 20 commits into from
Nov 5, 2023
Merged

feature: Import action #9364

merged 20 commits into from
Nov 5, 2023

Conversation

danharrin
Copy link
Member

  • Changes have been thoroughly tested to not break existing functionality.
  • New functionality has been documented or existing documentation has been updated to reflect changes.
  • Visual changes are explained in the PR description using a screenshot/recording of before and after.

@danharrin danharrin added the enhancement New feature or request label Oct 29, 2023
@pxlrbt
Copy link
Member

pxlrbt commented Oct 30, 2023

Shouldn't this be a separate Filament package? Especially since it introduces two migrations that not everybody will need?

@danharrin
Copy link
Member Author

The migrations need to be published, so it doesn't actually have a footprint on your app.

The reason why I don't want it as a plugin, is because there are no real dependencies apart from league/csv which everyone has already, and also we'd probably need two separate plugins, one for filament/actions and eventually one for filament/tables to handle the table version of this action, if that makes sense? So I dont mind it being in core, its quite a common use case

danharrin and others added 3 commits October 30, 2023 08:05
@danharrin @zepfietje
Update import.php
b803c38 
Co-authored-by: Zep Fietje <[email protected]>
@danharrin @zepfietje
Update import.php
5047d00 
Co-authored-by: Zep Fietje <[email protected]>
@danharrin @zepfietje
Update import.php
30a12d2 
Co-authored-by: Zep Fietje <[email protected]>
@danharrin @zepfietje
Update import.php
33ba015 
Co-authored-by: Zep Fietje <[email protected]>
@danharrin danharrin changed the base branch from 3.x to 3.1 October 30, 2023 10:59
@SebastiaanKloos
Copy link
Contributor

SebastiaanKloos commented Oct 30, 2023 •
edited
Loading

Import and export should be part of the core imo. Really cool API @danharrin. I like it 🚀

@danharrin danharrin requested a review from zepfietje October 30, 2023 20:42
@drbyte
Copy link
Contributor

drbyte commented Oct 31, 2023

Wonderful addition!

zepfietje
zepfietje requested changes Oct 31, 2023
View reviewed changes
Copy link
Member

@zepfietje zepfietje left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, apart from the Creating an importer docs section that's still missing.
Also, the icon alias should be added to the docs, @danharrin.

@zepfietje zepfietje added this to the v3 milestone Oct 31, 2023
@danharrin danharrin modified the milestones: v3, v3.1 Oct 31, 2023
@danharrin danharrin merged commit b8e1128 into 3.1 Nov 5, 2023
@danharrin danharrin deleted the import-action branch November 5, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Saifallak Saifallak Saifallak left review comments

@zepfietje zepfietje zepfietje requested changes

@awcodes awcodes Awaiting requested review from awcodes

@pxlrbt pxlrbt Awaiting requested review from pxlrbt

@ryangjchandler ryangjchandler Awaiting requested review from ryangjchandler

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
Roadmap
Archived in project
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@danharrin @pxlrbt @SebastiaanKloos @drbyte @Saifallak @zepfietje