canonical / react-components

A set of components based on Vanilla Framework
https://canonical.github.io/react-components
92 stars 51 forks source link

Slider input field has wrong width #1041

Closed lorumic closed 5 months ago

lorumic commented 6 months ago

This seems to be a problem introduced in the context of #1036 (version 0.49.0).

The slider input used to have a 5% width (verified checking out the commit tagged v0.48.0), and hence looked like this:

Screenshot from 2024-02-09 15:41:16

From version 0.49.0, it looks like this instead:

image

@petermakowski Do you know why this is happening?

petermakowski commented 6 months ago

Very strange as there's no CSS code that could affect this in this change. At a glance it looks like the order of CSS declarations has changed. I'll investigate.

petermakowski commented 6 months ago

@bartaz Some background info I managed to figure out so far: %vf-input-elements cannot be used without @include vf-b-forms, which includes global styles for input elements. https://github.com/canonical/vanilla-framework/blob/b4378c920f8bcb804db2ead64566ce3fa54636ae/scss/_base_forms.scss#L144

As soon as you add @include vf-b-forms to a component in order to extend %vf-input-elements, that as a result affects the order final CSS declarations and specificity which may alter styles in unrelated component.

Screenshots

styles from base forms end up having higher specificity than those of a slider

Google Chrome screenshot 001541@2x

after removal of @include vf-b-forms import in MultiSelect.scss - styles declarations have expected order https://github.com/canonical/react-components/pull/1042

Google Chrome screenshot 001543@2x

We could increase specificity of .p-slider__input alternatively, but the real fix would be to give an ability to just import base input styles (%vf-input-elements) without global styles.

bartaz commented 6 months ago

We could increase specificity of .p-slider__input alternatively, but the real fix would be to give an ability to just import base input styles (%vf-input-elements) without global styles.

@petermakowski Thanks for looking into this and explaining the context. I think the issue is that we don't include Vanilla base styles in React components and try to add styles on top of that. That will not work.

As mentioned here (https://vanillaframework.io/docs/customising-vanilla#importing-individual-components), to include individual compoents (or in our case, to build component styles on top of Vanilla), whole vf-base needs to be included (which includes vf-b-forms, among others). That's the only way to make sure all the necessary global and base styles are included in correct source order.

I don't know how to align that with React components. Obviously we don't want each component to separately include vf-base, but it seems in the React components styles we need the base included, so that components can extend anything from it.

bartaz commented 5 months ago

The problem seems to be deeper actually and we need to find a solution.

Currently when the MultiSelect component includes parts of Vanilla and uses @extend to build on top of them it seems to be creating a duplicate version of parts of Vanilla, messing up with source order and breaking things in unexpected ways.

https://github.com/canonical/react-components/blob/708f18999f1a0b8bb52e6e55b58e9b00d27ca759/src/components/MultiSelect/MultiSelect.scss#L1-L4

This is kind of one of the reasons why we ended up removing SCSS from the React components completely before. React components rely on Vanilla being in the project already. If on top of this we add components that have their own custom styling including Vanilla, it created multiple copies of Vanilla in the project, which is a huge issue.

github-actions[bot] commented 5 months ago

:tada: This issue has been resolved in version 0.50.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: