-
Notifications
You must be signed in to change notification settings - Fork 1
feat: create program area form #718
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
Conversation
…o mcm/feat-user-mgmt-table
* @param props.legend Legend (name) of the field set | ||
* @returns DIBBs styled fieldset component | ||
*/ | ||
export const FieldSet = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, trying to think if there's a way to use this in BaseFilter
as well (though the CSS is different).....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think no (or at least yet) since the styling is different
@@ -123,3 +125,45 @@ $table-border-radius-md: 0.5rem; | |||
text-decoration: none; | |||
} | |||
} | |||
|
|||
.dibbs-fieldset { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other places, you use the utility classes directly in className=
, and only leave the stylesheets for cases where utilities aren't available. Curious why you created dibbs-fieldset
instead of adding the styles with utility equivalents (i.e. display flex, border 1px, border color, etc.) to the inline className?
Edit: (I can see how grouping all the styles into a single class would be nice for clarity.....)
Also curious why you named it dibbs-fieldset
instead of something like form-fieldset
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's similar to how many of the component-y uswds classes set a lot of styles for themselves and children. Much easier on the end user to set one class and opt in to the opinionated style for that thing than need to re-create it on every item
dibbs-fieldset
just because at least for now it's the dibbs-styled fieldset and we do have other fieldsets in forms that aren't using this, so I wanted to be a bit more clear about what it was trying to achieve
} | ||
} | ||
|
||
.usa-checkbox__label { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be in custom-uswds-styles.scss
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it's a sub-field in .dibbs-fieldset
it can't really move on its own nicely. We could move all of .dibbs-fieldset
over there, but it's not really customizing a .usa-fieldset
, so that felt off to me - what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok, that sounds fine to me
); | ||
}; | ||
|
||
const keysToBoolean = <T extends object>(obj: T, val: boolean) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be stretching it, but wonder if this is a util (?) that we can use in Filters.tsx
as well...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's similar to what's happening there, but different enough and not beefy enough that I'm not sure it's worth abstracting out (yet)
…o mcm/feat-add-prog
PULL REQUEST
Summary
Add abstractions for form pages and for the program area forms and apply them to the create page.
Based on accessibility issues with the checkbox in the header, this was moved to select all/deselect buttons within the body of the category.
Related Issue
Fixes #709
Acceptance Criteria
As a single-page component:
Back to Program Management
button is presentAdd New Program Area
button available on program management homepageAdmin will only be able to add program area one at a time.
Confirmation page will come in a later ticket.
Additional Information
Before:

Blank form:

Filled out form:

After:

Error state:
