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

fix(frontend): Enhance Edition of Properties for a Node #362

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

Piv94165
Copy link
Contributor

@Piv94165 Piv94165 commented Jan 25, 2024

What

  1. When adding a property and confirming the row, the new property is not instantly visible in the table.
  2. After adding a property, confirming the row, and saving changes, the new property is visible in the table
  3. When deleting a property and confirming, the row is not instantly removed from the table.
  4. After deleting a property, confirming, and saving changes, the deleted property is not visible anymore
  5. Editing properties is working correctly if we only edit the value, otherwise it doesn't work well

Screenshot

EditionPropertiesWithoutPR1.mp4

Fixes bug(s)

EditionPropertiesWithPR1.mp4

I've changed the style of the table and removed pagination to help users to edit properties :
image

@Piv94165 Piv94165 linked an issue Jan 25, 2024 that may be closed by this pull request
5 tasks
@Piv94165 Piv94165 changed the title Fix(frontend) : Enhance Edition of Properties for a Node fix(frontend) : Enhance Edition of Properties for a Node Jan 25, 2024
@Piv94165 Piv94165 changed the title fix(frontend) : Enhance Edition of Properties for a Node fix(frontend): Enhance Edition of Properties for a Node Jan 25, 2024
@Piv94165 Piv94165 marked this pull request as ready for review January 25, 2024 13:43
@Piv94165 Piv94165 requested a review from a team as a code owner January 25, 2024 13:43
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

Works fine for me.

Although there are quirk, as we don't get appropriate error messages if the property name is not valid (we provoque a 500 error instead).
See #380 for that.

Copy link
Contributor

@eric-nguyen-cs eric-nguyen-cs left a comment

Choose a reason for hiding this comment

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

Thanks for solving the bugs, it's much more usable!
However, I have a style/UX nit: could you make it so the UI does not "jump" when we do edits/save changes? (I think that leaving some vertical space below the table for the "Save changes" button to appear should do it)
If you do not want to do it in this PR (to be fair, the footer vertical blank space should be applied to the App component), you can just create an issue

@Piv94165 Piv94165 merged commit 9d765d0 into main Feb 7, 2024
7 checks passed
@Piv94165 Piv94165 deleted the fix/node-property-deletion branch February 7, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Fix properties table for a node
3 participants