Disable save button if required fields are not populated #1505

roseeichelmann · 2024-12-10T05:31:34Z

Associated issues

Closes cityofaustin/atd-data-tech#20084

This actually ended up being kinda weird to implement, i had to really dig thru github and stack overflow before i found out how to get the edited values while still in edit mode and cause a re-render to enable the save button. Finally I found this github issue where someone had the same problem and a rly helpful comment describing how to handle our use case with useGridSelector

Testing

URL to test:

https://deploy-preview-1505--atd-moped-main.netlify.app/moped/

Steps to test:

On the project summary page, test adding a new subproject. The save button should be disabled until you select a project. Make sure the cancel and delete buttons work as well.
Test all the other datagrid tables too, Milestones, Funding, and Files. If you dont have the required fields filled out then the save should be disabled. BTW Funding has no required fields.
Test editing and deleting required fields, the save button should get disabled again.

Ship list

Code reviewed
Product manager approved
Product manager checked DB view dependencies
Product manager added to QA test script if applicable
- Added Try to save a row without the Milestone column populated. You should see the Required helper text and input highlighted in red. The save button should also be disabled. as a Milestones table test.

…on buttons and save is disabled until required fields are present

roseeichelmann · 2024-12-11T22:12:34Z

moped-editor/src/components/DataGridPro/DataGridActions.js

created a reuseable component to be used in all our data grid tables for the action buttons

roseeichelmann · 2024-12-11T22:13:06Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        renderCell: ({ id }) => (
+          <DataGridActions
+            id={id}
+            requiredFields={["file_name", "file_url", "file_type"]}


do yall like this or do you want to define requiredFields elsewhere in a var somewhere?

this feels good to me. If I were to start a new table that needed an inline form, I could imagine this prop making me stop to consider what fields need are required as soon as i added this component.

I briefly thought about a way to tie to the column config but that feels like over-optimization at this point imo. None of these inline forms have a long list of required fields since we've tried to minimize side-scrolling.

obligatory comment about non-stable object references: i think this is the slightest of anti-patterns versus declaring a const outside of the component 😇

ahh yeah that is a great point, John! could you update @roseeichelmann?

mddilley

This looks awesome! I tested each table, and I don't see any regressions. I also see that saving on click-away is blocked when a required field is missing. 👍

Thank you for the level up on introspecting the grid state too. I added two questions, and requested one change outside the scope of this issue. This is ready to 🚢!

moped-editor/src/components/DataGridPro/DataGridActions.js

moped-editor/src/views/projects/projectView/ProjectFunding/ProjectFundingTable.js

mddilley · 2024-12-12T16:19:56Z

moped-editor/src/components/DataGridPro/DataGridActions.js

This reusable component is so nice! ✨ Really cool to this common part of these tables extracted. 🧱

Also, thanks for matching this path up with the spot that Mateo began stashing DataGrid-related components. I know there are a lot of moving parts right now. 🙏

mddilley · 2024-12-12T16:25:19Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        renderCell: ({ id }) => (
+          <DataGridActions
+            id={id}
+            requiredFields={["file_name", "file_url", "file_type"]}


this feels good to me. If I were to start a new table that needed an inline form, I could imagine this prop making me stop to consider what fields need are required as soon as i added this component.

I briefly thought about a way to tie to the column config but that feels like over-optimization at this point imo. None of these inline forms have a long list of required fields since we've tried to minimize side-scrolling.

moped-editor/src/views/projects/projectView/ProjectMilestones.js

moped-editor/src/components/DataGridPro/DataGridActions.js

frankhereford

This is great - works perfect. Code looks good, but some of it is def above my pay grade. 🚢🚢🚢

I have one request -- and this is embarrassing -- I'm not sure how to nail down exactly where in the app this change needs to be made, but I figure y'all can real easy. Also - I'm pretty sure this behavior is not a result of this PR, so maybe this is a spin off thing, if y'all agree.

When I click on the cell, it gets this "focus" like behavior, and the enclosing cell gets this one pixel blue stroke around it that is out of place, imho. Like this:

This is the element, and it's this element that I'm struggling to point to in the application.

If you give that element a outline: none; CSS property, it's fixed. I'm curious what you and @mddilley think. Thanks!!

mddilley · 2024-12-12T18:20:22Z

@frankhereford thanks for the feedback! This is default styling and a feature of DataGridPro as seen in the CRUD example so I'd like to stick with the focus highlighting unless users have negative feedback about it.

frankhereford · 2024-12-12T18:28:45Z

@mddilley wrote:

This is default styling and a feature of DataGridPro

Sounds great to me. A last thought and forgive me, this is at the risk of feeling like i'm overly pushing back, but I don't mind the example you linked at all.

I think it's the fact that we have a stroke around the cell, and a stroke around what's in the cell, and we have no padding separating the two. It becomes unusual looking to me because the hierarchy of what contains what is unclear. I guess my take is that like borders shouldn't abut onto other borders of the same style.

In the screenshot above, I see a figure-8 of thin stroke, and is that a pair of cells next to each other or one containing the other? Being rhetorical here: Can I type in the second, smaller inpux box? Is that an input box, it kinda looks like one?

Again - i'm super cool with sticking with it -- i just wanted to better explain my observation. Thank you for the super quick response!!

johnclary

Good stuff—I couldn't get anything done without filling in those required fields. Very nice UX upgrade here! 🚢 🚢 🚢 🚢

moped-editor/src/views/projects/projectView/ProjectMilestones.js

johnclary · 2024-12-12T19:02:23Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        renderCell: ({ id }) => (
+          <DataGridActions
+            id={id}
+            requiredFields={["file_name", "file_url", "file_type"]}


obligatory comment about non-stable object references: i think this is the slightest of anti-patterns versus declaring a const outside of the component 😇

johnclary · 2024-12-12T19:07:20Z

moped-editor/src/components/DataGridPro/DataGridActions.js

+      }
+    }
+    return true;
+  });


very cool to see how this works and nice docstring 🙏

moped-editor/src/views/projects/projectView/ProjectFunding/ProjectFundingTable.js

mddilley · 2024-12-12T21:46:30Z

@frankhereford thanks for expanding about the focus outline colliding with the input, and now I'm seeing it! 🔍 🙏

We have inconsistent spacing from table to table so I captured making it uniform in cityofaustin/atd-data-tech#20171 and cityofaustin/atd-data-tech#20213 for when we consolidate and make reusable components to use across the tables.

roseeichelmann · 2024-12-12T22:12:42Z

@mddilley this is ready for re-review :))

mddilley

New commits look great, and I retested with the latest changes. Thanks for updating!! 🚀🚀🚀

roseeichelmann added 2 commits December 9, 2024 23:30

create reuseable component for datagrid tables that contains the acti…

ace1a13

…on buttons and save is disabled until required fields are present

use reuseable datagridactions component

1c19f1a

roseeichelmann added the WIP Work in progress label Dec 10, 2024

roseeichelmann added 7 commits December 10, 2024 16:56

add optional edit button to datagridactions component

cdbd675

make milestones table use actions component that disables save

2ad3387

clean up closing tag

c2f08e0

add comments

cad6b14

move file

6064881

use new actions component

9b069c1

change order of import

3577697

roseeichelmann commented Dec 11, 2024

View reviewed changes

roseeichelmann requested review from mddilley, chiaberry, johnclary, frankhereford, mateoclarke and tillyw December 11, 2024 22:13

add comment

686f3c2

mddilley removed the WIP Work in progress label Dec 12, 2024

mddilley approved these changes Dec 12, 2024

View reviewed changes

frankhereford approved these changes Dec 12, 2024

View reviewed changes

johnclary approved these changes Dec 12, 2024

View reviewed changes

roseeichelmann added 4 commits December 12, 2024 16:06

update deleteclick function call, use default prop, update int to num

1ebf8ba

create a required fields const, use a curried function

2a00082

add requiredfields const and make delete open a curried function

dbd7f52

add required fields const and make delete open a curried function

256e1a2

use helper text

05af360

roseeichelmann requested a review from mddilley December 12, 2024 22:12

mddilley approved these changes Dec 12, 2024

View reviewed changes

roseeichelmann merged commit 597d76f into main Dec 12, 2024
4 checks passed

roseeichelmann deleted the rose/20084_datagrid_disable_save branch December 12, 2024 22:48

mddilley mentioned this pull request Dec 19, 2024

Update Team table with required field validation cityofaustin/atd-data-tech#20357

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable save button if required fields are not populated #1505

Disable save button if required fields are not populated #1505

roseeichelmann commented Dec 10, 2024 •

edited by mddilley

Loading

roseeichelmann Dec 11, 2024

roseeichelmann Dec 11, 2024

mddilley Dec 12, 2024

johnclary Dec 12, 2024 •

edited

Loading

mddilley Dec 12, 2024

mddilley left a comment •

edited

Loading

mddilley Dec 12, 2024

mddilley Dec 12, 2024

frankhereford left a comment

mddilley commented Dec 12, 2024

frankhereford commented Dec 12, 2024 •

edited

Loading

johnclary left a comment

johnclary Dec 12, 2024 •

edited

Loading

johnclary Dec 12, 2024

mddilley commented Dec 12, 2024

roseeichelmann commented Dec 12, 2024

mddilley left a comment

Disable save button if required fields are not populated #1505

Disable save button if required fields are not populated #1505

Conversation

roseeichelmann commented Dec 10, 2024 • edited by mddilley Loading

Associated issues

Testing

Ship list

roseeichelmann Dec 11, 2024

Choose a reason for hiding this comment

roseeichelmann Dec 11, 2024

Choose a reason for hiding this comment

mddilley Dec 12, 2024

Choose a reason for hiding this comment

johnclary Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

mddilley Dec 12, 2024

Choose a reason for hiding this comment

mddilley left a comment • edited Loading

Choose a reason for hiding this comment

mddilley Dec 12, 2024

Choose a reason for hiding this comment

mddilley Dec 12, 2024

Choose a reason for hiding this comment

frankhereford left a comment

Choose a reason for hiding this comment

mddilley commented Dec 12, 2024

frankhereford commented Dec 12, 2024 • edited Loading

johnclary left a comment

Choose a reason for hiding this comment

johnclary Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

johnclary Dec 12, 2024

Choose a reason for hiding this comment

mddilley commented Dec 12, 2024

roseeichelmann commented Dec 12, 2024

mddilley left a comment

Choose a reason for hiding this comment

roseeichelmann commented Dec 10, 2024 •

edited by mddilley

Loading

johnclary Dec 12, 2024 •

edited

Loading

mddilley left a comment •

edited

Loading

frankhereford commented Dec 12, 2024 •

edited

Loading

johnclary Dec 12, 2024 •

edited

Loading