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

More form input types #2542

Merged
merged 11 commits into from
Feb 13, 2023
Merged

More form input types #2542

merged 11 commits into from
Feb 13, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Feb 8, 2023

Needed for #2435

The new designs have a new type of input that looks like

image

whereas our existing inputs look like

image

This PR adds a new variant prop to a few of the custom form fields that can either be stacked or inline in order to support both input types.

Code Changes

  • Split form components into 3 common parts:
    • Label
    • Input
    • Error message
  • Refactor all components to use at least the common Label and Error message
  • Add stacked and inline variants. By splitting the components into subcomponents, it makes it easier to make the component return the same thing, just visually different
  • Refactored CustomSelect and CustomMultiSelect to be one component that takes an isMulti option

Steps to Confirm

  • Click through various forms and make sure they still look the same as before. Nothing uses the new stacked variant yet, and the forms default to inline, so nothing should look different.

Pre-Merge Checklist

Description Of Changes

Inline vs stacked CustomTextInput
image

Inline vs stacked CustomTextArea
image

Inline vs stacked CustomSelect
image

@codecov
Copy link

codecov bot commented Feb 8, 2023

Codecov Report

Base: 88.66% // Head: 85.73% // Decreases project coverage by -2.93% ⚠️

Coverage data is based on head (829bb8e) compared to base (924aaf2).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2542      +/-   ##
==========================================
- Coverage   88.66%   85.73%   -2.93%     
==========================================
  Files         331      285      -46     
  Lines       16099    15476     -623     
  Branches     4469     1945    -2524     
==========================================
- Hits        14274    13269    -1005     
- Misses       1668     1832     +164     
- Partials      157      375     +218     
Impacted Files Coverage Δ
.../ops/service/messaging/message_dispatch_service.py 45.45% <0.00%> (-29.30%) ⬇️
...ides/api/ops/email_templates/get_email_template.py 60.00% <0.00%> (-20.00%) ⬇️
src/fides/api/ctl/routes/util.py 64.51% <0.00%> (-16.13%) ⬇️
src/fides/api/ops/db/base_class.py 71.42% <0.00%> (-15.24%) ⬇️
src/fides/core/annotate_dataset.py 21.12% <0.00%> (-14.49%) ⬇️
.../service/privacy_request/request_runner_service.py 74.29% <0.00%> (-11.71%) ⬇️
src/fides/api/ops/schemas/api.py 80.00% <0.00%> (-11.67%) ⬇️
src/fides/lib/db/session.py 78.57% <0.00%> (-11.09%) ⬇️
.../fides/api/ops/service/connectors/sql_connector.py 76.14% <0.00%> (-10.64%) ⬇️
src/fides/core/config/notification_settings.py 90.00% <0.00%> (-10.00%) ⬇️
... and 254 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@allisonking allisonking marked this pull request as ready for review February 9, 2023 00:31
Copy link
Contributor

@ssangervasi ssangervasi left a comment

Choose a reason for hiding this comment

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

I value your Input

@cypress
Copy link

cypress bot commented Feb 13, 2023

Passing run #43 ↗︎

0 3 0 0 Flakiness 0

Details:

Merge 829bb8e into ccdeb2a...
Project: fides Commit: a26129c261 ℹ️
Status: Passed Duration: 00:36 💡
Started: Feb 13, 2023 9:10 PM Ended: Feb 13, 2023 9:10 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@allisonking
Copy link
Contributor Author

Failing test is unrelated

@allisonking allisonking merged commit a0a2984 into main Feb 13, 2023
@allisonking allisonking deleted the aking/more-form-input-types branch February 13, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants