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

feat: Implement Data frame editor.WF-66 #529

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

feat: Implement Data frame editor.WF-66 #529

wants to merge 2 commits into from

Conversation

madeindjs
Copy link
Collaborator

@madeindjs madeindjs commented Aug 20, 2024 •
edited
Loading

Implement behavior to edit a dataframe started in #485 (FabienArcellier#63)

Screencast.from.2024-09-11.21-18-05.mp4

For now, it only allows editing existing cells, but it'll be possible to add/delete some rows in the future.

Technically:

  1. I introduced a new component CoreDataframeCell which re-render CoreDataframeCellText[Text|Number|Boolean|Unknown]
  2. those component emits on changes, this event is cached by CoreDataframe
  3. CoreDataframe handle this event and call a new hook useDataFrameValueBroker which handle the update and take care of updating the arquero table

@madeindjs madeindjs self-assigned this Aug 20, 2024
@madeindjs madeindjs force-pushed the data-frame-editor branch 2 times, most recently from de89bef to 6515937 Compare September 7, 2024 22:42
@madeindjs madeindjs marked this pull request as ready for review September 11, 2024 19:22
@madeindjs madeindjs changed the title feat: Implement Data frame editor feat: Implement Data frame editor.WF-26 Sep 11, 2024
@madeindjs madeindjs changed the title feat: Implement Data frame editor.WF-26 feat: Implement Data frame editor.WF-66 Sep 11, 2024
@FabienArcellier
Copy link
Collaborator

FabienArcellier commented Sep 14, 2024 •
edited
Loading

I didn't find how to move to this page while navigating in the hello application?
image

2 proposals :

  • move the editable dataframe into tabulation instead of it's own page
  • create a section with a link into tab to keep it in a separate page

For creating a section with link into tab, I would recommand those changes :
image
image

@FabienArcellier
Copy link
Collaborator

I have been looking for adding a line for few seconds. I am not sure what the good option is.

An option is to have something more consistent with the action menu. Maybe the same format than dropdown on the bottom left.

image

@FabienArcellier
Copy link
Collaborator

Boolean should use the same cursor than other type to show you can edit

image
image

@FabienArcellier
Copy link
Collaborator

(not related to this PR directly) The csv export add a column __internal_arquero_id. I think this column should not be here.

image

@FabienArcellier
Copy link
Collaborator

There is a bug after using the search control. The edit will change all the row. I haven't investigate the cause yet.

Peek 2024-09-14 08-16

FabienArcellier
FabienArcellier reviewed Sep 14, 2024
View reviewed changes
apps/hello/main.py Outdated Show resolved Hide resolved
@ramedina86
Copy link
Collaborator

For now, it only allows editing existing cells, but it'll be possible to add/delete some rows in the future.

I believe this is inaccurate?

@ramedina86
Copy link
Collaborator

Can you add a better looking plus (+) button? With material symbols and using the WDS button

@madeindjs
Copy link
Collaborator Author

@ramedina86 , do you mean to restore the buttons a bit like I did before ? If yes, using <WdsButton size="small" /> give me this result, WDYT ?

secondary tertiarry
image image

@FabienArcellier FabienArcellier mentioned this pull request Sep 26, 2024
Open
@ramedina86
Copy link
Collaborator

That looks good to me, why was it changed? What's the downside?

@FabienArcellier FabienArcellier added the enhancement New feature or request label Sep 26, 2024
@madeindjs
Copy link
Collaborator Author

That looks good to me, why was it changed? What's the downside?

I implemented it @ramedina86

@FabienArcellier FabienArcellier mentioned this pull request Oct 4, 2024
Open
@madeindjs
feat(ui): put Dataframe editor behind feature flag - WF-66
f379733 
Put the new dataframe editor behind the flag `dataframeEditor`. So to
activate the new feature, you need to add the flag in `main.py` like
this:

```py
wf.Config.feature_flags.append("dataframeEditor")
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabienArcellier FabienArcellier FabienArcellier left review comments

Assignees

@madeindjs madeindjs

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@madeindjs @FabienArcellier @ramedina86