feat(Slider): add experimental slider by emyarod · Pull Request #1683 · carbon-design-system/carbon-components-react

emyarod · 2018-12-21T00:58:40Z

Closes IBM/carbon-components-react#1682

This PR will add the new experimental slider changes

related https://github.com/IBM/carbon-components-react/issues/1681

netlify · 2018-12-21T01:00:53Z

Deploy preview for carbon-components-react ready!

Built with commit 6d88eed

https://deploy-preview-1683--carbon-components-react.netlify.com

asudoh · 2018-12-22T06:02:06Z

  }

-  UNSAFE_componentWillReceiveProps(nextProps) {
-    if (!sliderValuePropSync && !isEqual(nextProps, this.props)) {


The removal here seems a breaking change, please see: #1370

asudoh

Great progress @emyarod! Great to get in sync with TJ wrt the markup change.

asudoh · 2018-12-22T06:04:28Z

            <TextInput
              type={inputType}
-              id="input-for-slider"
+              id={`input-for-slider${id}`}


We tend to use ${id} as a prefix instead of a suffix (e.g. https://github.com/IBM/carbon-components-react/blob/v6.39.0/src/components/Select/Select.js#L31) - Wondering if it'd make sense to align the convention.

asudoh

LGTM 👍 if the change works with v9 style. Do you think this is still blocked by #1681? Thanks @emyarod !

emyarod · 2019-01-08T14:49:09Z

@asudoh probably is not blocked but the topic discussed in #1681 is still something to consider in the future I believe

emyarod · 2019-01-09T15:57:54Z

@asudoh the visual bug we are seeing in the React implementation is caused by the problem addressed in https://github.com/IBM/carbon-components-react/issues/1681 and #1684

Our React <TextInput> includes a form item/<FormItem> wrapper which affects the styles that should be applied to the input field

so we end up rendering

<div class="bx--form-item">
  <input>
</div>

rather than just

<input>

asudoh · 2019-01-09T21:21:30Z

@emyarod As we discussed the slider element order issue is caused by https://github.com/IBM/carbon-components/pull/1315/files#diff-f8f1a0577791a85d255a9cdf4e2f791fR30 - Did you see it differently, or were you talking about another issue? Any more background?

emyarod · 2019-01-10T18:07:45Z

@asudoh from what I can tell, the issue is caused by our <TextInput> component's wrapper div.

https://github.com/IBM/carbon-components-react/blob/master/src/components/Slider/Slider.js#L464

https://github.com/IBM/carbon-components-react/blob/master/src/components/TextInput/TextInput.js#L77

removing this wrapper causes the input box to be positioned correctly

I believe this line is also relevant in why a wrapper div would ruin the order of the elements https://github.com/IBM/carbon-components/pull/1315/files#diff-f8f1a0577791a85d255a9cdf4e2f791fR38

asudoh · 2019-01-10T20:47:37Z

Thank you for the details @emyarod! I think the last link explains the issue the best. That said, when you remove the wrapper div, would you try adding display:flex to the new parent node in DOM inspector and see what happens?

emyarod · 2019-01-11T19:28:14Z

@asudoh it looks like the new parent node already contains display: flex after removing the wrapper div, since the new parent node is .bx--slider-container

asudoh · 2019-01-12T00:43:35Z

@emyarod Thank you for taking a look - Further investigation revealed that I was confused div.bx--form-item around text input vs. div.bx--form-item around slider, my apologies.

From that, I see two approaches to fix the problem:

Direct usage of <input> instead of using <TextInput>
Adding a CSS class for ordering <TextInput> within div.bx--form-item around slider

Thoughts...?

emyarod · 2019-01-14T16:31:32Z

@asudoh I am leaning towards option 2 right now, but eventually I would like to address this in a future release after reopening the discussion on #1684

emyarod · 2019-01-16T21:23:12Z

blocked by carbon-design-system/carbon#1640

asudoh

LGTM 👍 - Thanks @emyarod!

This reverts commit cc0c674.

This reverts commit f15b494.

emyarod · 2019-01-22T17:50:00Z

updated to include changes from carbon-design-system/carbon#1640

carbon-bot · 2019-01-22T23:36:26Z

🎉 This PR is included in version 6.81.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

emyarod added the status: blocked 🙅‍♀️ label Dec 21, 2018

asudoh reviewed Dec 22, 2018

View reviewed changes

emyarod force-pushed the 1682-experimental-slider branch 2 times, most recently from d09f186 to 37f63fa Compare January 2, 2019 19:20

emyarod force-pushed the 1682-experimental-slider branch from 37f63fa to 16cb84a Compare January 7, 2019 16:36

emyarod changed the title ~~feat(Slider): add experimental slider~~ [WIP] feat(Slider): add experimental slider Jan 7, 2019

emyarod force-pushed the 1682-experimental-slider branch from c18724d to aff2cd8 Compare January 7, 2019 21:17

asudoh reviewed Jan 8, 2019

View reviewed changes

emyarod added status: WIP 👩‍💻 and removed status: blocked 🙅‍♀️ labels Jan 8, 2019

emyarod force-pushed the 1682-experimental-slider branch from aff2cd8 to 67f271d Compare January 9, 2019 15:58

emyarod added the status: blocked 🙅‍♀️ label Jan 9, 2019

emyarod mentioned this pull request Jan 9, 2019

fix(slider): remove unused and unclosed div carbon-design-system/carbon#1609

Merged

emyarod force-pushed the 1682-experimental-slider branch from 67f271d to bd70265 Compare January 11, 2019 19:25

emyarod force-pushed the 1682-experimental-slider branch from bd70265 to 39880cf Compare January 14, 2019 16:30

This was referenced Jan 15, 2019

Favor markup rearrangement over order CSS property in experimental slider carbon-design-system/carbon#1639

Closed

refactor(slider): rearrange experimental markup instead of CSS order carbon-design-system/carbon#1640

Merged

emyarod changed the title ~~[WIP] feat(Slider): add experimental slider~~ feat(Slider): add experimental slider Jan 16, 2019

emyarod removed the status: WIP 👩‍💻 label Jan 16, 2019

emyarod force-pushed the 1682-experimental-slider branch 2 times, most recently from 4184c4e to 9c97e21 Compare January 21, 2019 15:57

asudoh approved these changes Jan 22, 2019

View reviewed changes

asudoh added status: one more review 👀 status: ready for review 👀 and removed status: blocked 🙅‍♀️ labels Jan 22, 2019

emyarod added 12 commits January 22, 2019 11:37

feat(Slider): match experimental markup

d4502c0

refactor(Slider): remove uncalled lifecycle method

b82f5b8

fix(Slider): declare correct prop types

176caaa

fix(Slider): use correct class name

1329396

fix(Slider): avoid duplicate IDs

f4f3c18

docs(Slider): set default input type

70789a8

Revert "refactor(Slider): remove uncalled lifecycle method"

4312147

This reverts commit cc0c674.

Revert "fix(Slider): avoid duplicate IDs"

7bf4da3

This reverts commit f15b494.

fix(Slider): avoid duplicate IDs

552fc9c

fix(Slider): add missing className

ae24938

chore: bump carbon-components version

ecdcea0

fix(Slider): match vanilla markup

6d88eed

emyarod force-pushed the 1682-experimental-slider branch from 9c97e21 to 6d88eed Compare January 22, 2019 17:48

joshblack approved these changes Jan 22, 2019

View reviewed changes

asudoh merged commit 5d4b040 into carbon-design-system:master Jan 22, 2019

carbon-bot added the released label Jan 22, 2019

emyarod deleted the 1682-experimental-slider branch January 23, 2019 16:19

emyarod mentioned this pull request Apr 30, 2019

Form spacing inconsistencies carbon-design-system/carbon#2343

Closed

Conversation

emyarod commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asudoh Dec 22, 2018

Choose a reason for hiding this comment

Uh oh!

asudoh left a comment

Choose a reason for hiding this comment

Uh oh!

asudoh Dec 22, 2018

Choose a reason for hiding this comment

Uh oh!

asudoh left a comment

Choose a reason for hiding this comment

Uh oh!

emyarod commented Jan 8, 2019

Uh oh!

emyarod commented Jan 9, 2019

Uh oh!

asudoh commented Jan 9, 2019

Uh oh!

emyarod commented Jan 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asudoh commented Jan 10, 2019

Uh oh!

emyarod commented Jan 11, 2019

Uh oh!

asudoh commented Jan 12, 2019

Uh oh!

emyarod commented Jan 14, 2019

Uh oh!

emyarod commented Jan 16, 2019

Uh oh!

asudoh left a comment

Choose a reason for hiding this comment

Uh oh!

emyarod commented Jan 22, 2019

Uh oh!

carbon-bot commented Jan 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emyarod commented Dec 21, 2018 •

edited

Loading

netlify Bot commented Dec 21, 2018 •

edited

Loading

emyarod commented Jan 10, 2019 •

edited

Loading