feat(cds-fluid-form): new wc component#21951
Conversation
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21951 +/- ##
==========================================
- Coverage 95.13% 95.10% -0.03%
==========================================
Files 548 549 +1
Lines 45602 45636 +34
Branches 6576 6578 +2
==========================================
+ Hits 43382 43403 +21
- Misses 2090 2103 +13
Partials 130 130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Just a heads up for visual review, the story controls have made it really easy to see how the fluid components behave together with the different states so I've noticed a bunch of visual problems with some of the fluid components in React/WC that would likely need issues to be created if they have not been already (e.g. FluidNumberInput & FluidDatePicker |
There was a problem hiding this comment.
@maradwan26 This is looking really good! Just a couple comments below:
There are some smaller visual bugs, which you mentioned, in Fluid Form in general that already existed in React, so I will make a separate issue for that. It's mostly regarding things per input or specifically the fluid form modal. Let me know if anything below needs to be a separate issue.
Default story
Fluid form on page
- The horizontal borders between the first and second rows in the form should be
1pxinstead of 2px. - Number input component: In Firefox, when the number input field receives focus, there is a glitchy half border around the - control. This seems to only be a Firefox problem.
- Disabled prop: When the prop is set to true, some of the borders between components disappear and the bottom border on number input is incorrect. They all should be
$border-disabled. - Invalid and Warning props: There are some issues with the alignment of these invalid and warning states. Is this a current problem in React? I couldn’t see it because I don’t have access to the prop in React. I can move this to a separate issue if it makes more sense to.
Could you clarify a bit more on this? On my end I see 1px in Dev Tools
This is a problem with the FluidNumberInput itself, but for me its on the "+" when I hover. I'm also seeing this in React and in both Chrome and Firefox 
I think this can be put in a separate issue since this is an issue outside of the fluid-form
The fluid number input bug originates all the way from the default number-input it just gets inherited here. I believe a fix for the regular number-input should fix this one? Unless there should be a different border for the fluid variant which kinda goes into my next questions regarding Regarding the borders when in For example, right now all the fluid-components have a bottom border of I'm assuming for the non-fluid variants,
I added controls to the React story in this PR so that all the states are visible: https://deploy-preview-21951--v11-carbon-react.netlify.app/?path=/story/components-fluid-components-fluidform--default And yeah, this is also an issue with React. The same goes for the |
There was a problem hiding this comment.
Stroke inconsistencies
The horizontal borders between the first and second rows in the form should be 1px instead of 2px.
Interesting, I am not seeing this anymore in any browser. So ignore this. 😄
Fluid number input bugs
Number input component: In Firefox, when the number input field receives focus, there is a glitchy half border around the - control. This seems to only be a Firefox problem.
I made a separate issue for the focus/hover number input problems (#22055).
Disabled state
All the borders should be set to $border-subtle according to our website guidance. I noticed in Figma we have a couple bugs around this, some of the fluid inputs don't even have bottom borders in the disabled state, so I am going to make a follow up issue in our kit repo for that.
I also see what you mean about the stemming disabled issue coming from the normal number input.
I went ahead and made a separate issue to tackle the number input disabled problem and the fluid form input problem (#22056).
Invalid / Warning states
I added controls to the React story in this PR so that all the states are visible: https://deploy-preview-21951--v11-carbon-react.netlify.app/?path=/story/components-fluid-components-fluidform--default
Thanks, yes so this is a problem stemming from React. I made a separate issue (#22054) to tackle these states for React and WC because there are many bugs with it.
laurenmrice
left a comment
There was a problem hiding this comment.
Approving because for the bugs I did find, I made separate issues for them because there were already problems for them coming from React. I also want to do a separate issue on the fluid modal form overall. Everything else looks good.
|
@laurenmrice Thank you! For #22056 I rephrased it to be more specific to the the Fluid components instead of the Fluid form. Just for added specificity since the fluid-components should still have the |
a5bc2c1
* feat(cds-fluid-form): new wc component * fix(fluid-components): add flex style * fix: hide modal-container overflow * feat: sync stories * feat(fluidform): storycontrols in react and wc * fix(fluid-password-input): remove excess bottom padding * test: add tests * fix: datepicker controls * chore: clean up imports * fix: passwordinput default alignment --------- Co-authored-by: Heloise Lui <71858203+heloiselui@users.noreply.github.com>
|
Hey there! v11.107.0 was just released that references this issue/PR. |
Closes #20341
Closes #20915
Closes #21950
Adds a new component
cds-fluid-formto@carbon/web-componentsalong with its respective test cases. Also adds controls to this components story in both React and WC storybooks. Fixes some minor form-related styling issues with some fluid components in WCChangelog
New
cds-fluid-formand its respective styles, tests, mdx, and storycds-fluid-date-pickernot being implemented in WC yet, I've added a TODO comment to address this once that's doneChanged
flex: 1 1 auto;not being set in their stylesheetcds-fluid-date-pickeronce it gets implementedcds-fluid-password-inputhaving excess padding at the bottomcds-password-input's default tooltip alignment value with ReactFluidTextInputwith type="password", they have been replaced withFluidPasswordInputTesting / Reviewing
cds-fluid-formshould render as expected in Web ComponentsPR Checklist
As the author of this PR, before marking ready for review, confirm you:
More details can be found in the pull request guide