[slider] General cleanup and add classes prop for unstyled by mnajdova · Pull Request #23569 · mui/material-ui

mnajdova · 2020-11-16T13:23:26Z

This PR extracts some changes from #23308. The following changes are being made:

add classes prop to the SliderUnstyled
updated buildAPI to reflect the definition in the classes for the CSS section in the api page
adds helper tests for the unstyled package
removed unnecessary component ValueLabelStyled and moved the styles to the ValueLabelStyled "slot" component.
renamed ValueLabelUnstyled to SliderValueLabelUnstyled and reverted back some changes from the original implementation

mui-pr-bot · 2020-11-16T13:25:59Z

@material-ui/lab: parsed: -0.13% 😍, gzip: -0.09% 😍

Generated by 🚫 dangerJS against 8067cb8

mnajdova · 2020-11-16T13:29:20Z

-    prettierConfigPath,
-  });
+
+  if (!nonJSSComponent) {


For the components generated with withStyled the info and descriptions are gathered from the component's static styles field. However, for the unstyled & the components generated with styled() this info does not exists, but is taken from the ts definition of the classes prop. That is why we do not need to update the annotation for the classes definition for these components.

mnajdova · 2020-11-16T13:30:02Z

  // styled components does not have the options static
-  const styledComponent = !component?.default?.options;
-  if (styledComponent) {
+  const nonJSSComponent = !component?.default?.options;


This name better reflects the condition as it is true for both components generated with the styled utility, as well as the unstyled components

oliviertassinari

Great! Much better with a different pull request :)

oliviertassinari · 2020-11-16T14:25:16Z

+
+| Rule name | Global class | Description |
+|:-----|:-------------|:------------|
+| <span class="prop-name">root</span> | <span class="prop-name">.root-1</span> | Class name applied to the root element. 


⚠️ .root-1

Good catch! The reason for it is that we don't have name for the SliderUnstyled :( (MuiSlider). We have few options of how to solve this

generate this name from the component name for the unstyled components -> ComponentNameUnstyled -> MuiComponentName

bring back the json only for this prop

add a static field on the component for it.

I would vote for the first option.

Here are the changes - 11e29df Let me know if you'd like this solved some other way, but I think this should be fine. Cc @eps1lon you may have an opinion on this too

I guess it depends. If we export a classes object will all the default global class names, then the problem is solved, we can read it. Otherwise 1. sounds better.

I think it's the best if the description are kept with the types definition, as they are for the other props. Exporting classes object can be complementary in my opinion.

oliviertassinari · 2020-11-16T14:27:26Z

  } catch (err) {
-    // Do nothing for now if the file doesn't exist
-    // This is still not supported for all components
+    // If the JSON file doesn't exists try extracting the info from the TS definition


Is the JSON file still necessary?

We won't after I add the classes prop on the SliderStyled too. Wanted to do the changes step by step. That will be my next PR (with that one we can drop this section and remove the JSON files

My bad, the prop already exists there, will do the necessary updates with this PR

Creating follow up PR for removing the JSON files

I've decided to include the changes as part of this PR - b254b2c as it wasn't related to the styled component.

Fixed whitespaces too - ec98f8d Seems like this is solid and we can remove the JSON files. The only info there left required was the name which can be generated easily by the filename.

Co-authored-by: Olivier Tassinari <[email protected]>

…jdova/material-ui into feat/slider-unstyled-cleanup

eps1lon

docs-components-slider-styled/CustomizedSlider.png looks off: https://deploy-preview-23569--material-ui.netlify.app/components/slider-styled/#customized-sliders

The label container isn't even visible for me on Chrome 86 Ubuntu 20.04:

mnajdova · 2020-11-16T16:41:31Z

docs-components-slider-styled/CustomizedSlider.png looks off: https://deploy-preview-23569--material-ui.netlify.app/components/slider-styled/#customized-sliders

The label container isn't even visible for me on Chrome 86 Ubuntu 20.04:

Isn't it suppose to look like that?

I don't see difference with the v4 CustomizedSlider
https://material-ui.com/components/slider/#customized-sliders

oliviertassinari · 2020-11-16T17:01:14Z

It looks OK in the demos, but not so much in the visual regression tests:

mnajdova · 2020-11-16T17:03:59Z

It looks OK in the demos, but not so much in the visual regression tests:

Just saw it and pished a fix for it - 1174a6b Let me know what you think

We cannot rely on slot components to define custom selectors, as if we are styling the core component it's not predictable which ones will be written first. I moved these selectors to the root component so this can be always correct. Not sure if it is the best fix, but the only one I see that makes sense.. This would open questions as to whether these slots should also be exposed, but I wouldn't go that far..

…nstyled-cleanup

mnajdova added 2 commits November 16, 2020 14:16

extracted changes

2f12e2f

fix

471c1c8

mnajdova mentioned this pull request Nov 16, 2020

[slider] Replace core Slider with SliderStyled #23308

Merged

improved name of variable

5751d0e

mnajdova commented Nov 16, 2020

View reviewed changes

github-actions Bot mentioned this pull request Nov 16, 2020

Update snapshots for pr/23569 eps1lon/mui-scripts-incubator#1375

Closed

mnajdova added 2 commits November 16, 2020 15:01

picked up latest changes from Slider test

b3d22a5

lint

620e447

oliviertassinari reviewed Nov 16, 2020

View reviewed changes

oliviertassinari added scope: slider Changes related to the slider. package: @mui/base Specific to @mui/base (legacy). labels Nov 16, 2020

Update docs/scripts/buildApi.ts

5ccd372

Co-authored-by: Olivier Tassinari <[email protected]>

oliviertassinari changed the title ~~[SliderUnstyled] General cleanup and add classes prop~~ [Slider] General cleanup and add classes prop for unstyled Nov 16, 2020

mnajdova added 2 commits November 16, 2020 15:40

fixed classes generated

11e29df

Merge branch 'feat/slider-unstyled-cleanup' of https://github.com/mna…

159a887

…jdova/material-ui into feat/slider-unstyled-cleanup

oliviertassinari approved these changes Nov 16, 2020

View reviewed changes

eps1lon reviewed Nov 16, 2020

View reviewed changes

Comment thread packages/material-ui-lab/src/SliderStyled/SliderStyled.test.js Outdated

eps1lon approved these changes Nov 16, 2020

View reviewed changes

fixed generation of proptypes and docs:api

05acd84

eps1lon requested changes Nov 16, 2020

View reviewed changes

tests

a9a1104

mnajdova requested a review from eps1lon November 16, 2020 16:45

fixed specificity issue

1174a6b

mnajdova added 2 commits November 16, 2020 18:05

Merge branch 'feat/slider-styled-add-classes-prop' into feat/slider-u…

b254b2c

…nstyled-cleanup

prettier

9ed02bd

oliviertassinari approved these changes Nov 16, 2020

View reviewed changes

mnajdova added 2 commits November 16, 2020 18:18

fixed whitespaces in the end

ec98f8d

lint

9e3a5eb

eps1lon approved these changes Nov 16, 2020

View reviewed changes

github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Nov 16, 2020

Merge branch 'next' into feat/slider-unstyled-cleanup

949fe94

github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Nov 16, 2020

mnajdova added 2 commits November 16, 2020 22:50

reverted some changes

ea65bc8

fixed window issue with new lines

0f4ddfb

mnajdova commented Nov 16, 2020

View reviewed changes

Comment thread docs/scripts/buildApi.ts Outdated

mnajdova added 5 commits November 17, 2020 00:03

added comment

807600b

reverted changes for windows

fa64f09

reverted changes

99843e0

reverted changes

eda5a01

lint

8067cb8

mnajdova merged commit 8fd056e into mui:next Nov 17, 2020

eps1lon mentioned this pull request Nov 17, 2020

Revert "[docs] API pages i18n" #23581

Merged

oliviertassinari changed the title ~~[Slider] General cleanup and add classes prop for unstyled~~ [slider] General cleanup and add classes prop for unstyled May 9, 2026

Uh oh!

Uh oh!

Conversation

mnajdova commented Nov 16, 2020

Uh oh!

mui-pr-bot commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova Nov 16, 2020 • edited by oliviertassinari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eps1lon left a comment

Choose a reason for hiding this comment

Uh oh!

mnajdova commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mnajdova commented Nov 16, 2020

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mui-pr-bot commented Nov 16, 2020 •

edited

Loading

oliviertassinari Nov 16, 2020 •

edited

Loading

mnajdova Nov 16, 2020 •

edited by oliviertassinari

Loading

oliviertassinari Nov 16, 2020 •

edited

Loading

mnajdova commented Nov 16, 2020 •

edited

Loading

oliviertassinari commented Nov 16, 2020 •

edited

Loading