[Tidy] Consolidate gaps in selectors and apply new designs to sliders

[huong-li-nguyen]

Description

• Consolidate gap between label and selectors
• Apply new designs to Slider and RangeSlider and tidy code
• Fix visual bug in Accordion (not enough space between last child and separating line)
• Note: Remove the increase/decrease button next to Slider/RangeSlider input fields (as same functionality can be achieved via the slider handles)

New designs for Slider/RangeSlider have several advantages

• Reduces lots of custom CSS classNames and core code changes
• Text input fields have been previously cut-off if numbers were longer than a certain width

We can still update (if desired):

• gap between label and input control
• gap between controls

Screenshot

Just run the dev example in this PR:

There is a gap between the hover color and the separating line now:

Notice

• [x] I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
  • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
  • I have not referenced individuals, products or companies in any commits, directly or indirectly.
  • I have not added data or restricted code in any commits, directly or indirectly.

[huong-li-nguyen]

Should we pay more attention to it? In my opinion, the distinction between single-valued and multi-valued Dropdown text color seems inconsistent.

Hey @petar-qb,

I just double-checked, and everything is according to the designs 👍

The coloring also makes more sense if you think about when we use the primary and secondary colors. The secondary color is our general color for text if you don't want to emphasize something (e.g., selector options, labels, placeholder text, etc.). We use the primary color to emphasize something, e.g., user-written input, selected values, etc.

I understand your confusion regarding the multi-dropdown multi and single values. In the single-select, a primary color is used because that's the selected value. In the multi-select, I don't know for sure, but it's our designer's choice according to Figma. I assume they didn't use the primary color there, as the tags are sufficiently visual to represent a "selection"? However, there are also some inconsistencies in the Figma designs. So here I think we could change it to text-primary. The rest is already consistent 👍

I just implemented that change - will let Pruthvi double-check the Figma/Storybook designs when he's back.

[huong-li-nguyen]

• gap between controls - looks ok. The sliders look a little squashed vertically in the control panel compared to my personal taste, but I don't know if we can make that any better while keeping spacing consistent and not making the gap between non-slider controls too large? Because the non-slider ones look good how they are. So don't change this just based on my personal preference, because the current solution looks ok to me

No, I agree with you. In the original Figma designs, the sliders also have a more significant gap than the other input components. I did increase the top/bottom padding of the actual slider-track now 👍 . This way, the gaps between the labels and the input components remain the same, but the sliders have more space. I am still deviating from the Figma designs, though, as the gaps there are even more significant, so I could increase the padding even more if we want to replicate it exactly. But I found it looks a bit disproportional to the other components then. Right now, it still looks fine.

Just one thing: have you tested the behavior of the sliders with marks explicitly set to something other than None? This is worth manually trying because I found some buggy behavior with the old designs, and I don't see it tested anywhere in the examples here.

I did test it manually before, and setting the marks worked fine! But I am glad you mentioned it because the drawn marks are a bit tricky as it depends not only on the marks argument but a combination of steps/marks (one will have precedence over the other in some cases). I remember Nadija testing all of the use cases manually once. However, I double-checked and realized we only cover some use cases in our unit tests. Hence, for some combinations, the wrong CSS class name was applied.

I've added a dev example here: https://github.com/mckinsey/vizro/pull/336/commits/14f6aa9a77b78d1d66a5231ebf64e9500f9d8d2a

And extended our unit tests here to capture the remaining cases (as I had to remember again what these are 😅): https://github.com/mckinsey/vizro/pull/336/commits/304d1c22477a1ededc238a1299b2aaf91c4d0614

[huong-li-nguyen]

Not going to look at code as you suggested, but just visual inspection. I think this looks really good, thanks for doing this! I just spotted one small thing:

Yes! I've noticed that as well, but I wouldn't include a fix in this PR as this one should really just consolidate the input controls and that bug existed before. I did take a quick look into this one a few days ago, but then decided that this is worth a separate issue ticket, because it does not seem to be a CSS issue.

If you e.g. provide a large number or step sizes of 0.5, they do not get cut-off (which was actually the case before this PR, but I extended the max-width). Weirdly, in some cases they do get cut off as in the example you have shown. I think it might be the underlying react component and maybe the fact that there is no data in the year XXXX.5 also plays a role. However, someone needs to properly investigate this (running a pure dash app etc.), as extending the width for the input field does not fix it. So I don't think it's a pure CSS issue, but I am not 100% sure.