Form input/select/textarea parameters put to variables

[nastiatikk]

Status: Ready for Review Owner: Anastasia Tikk Reviewers: @jeffkamo @avelinet @cole-sanderson @kpeatt @ry5n @mlworks

Changes

• Form elements (input, textarea, select) parameters put to the variables. It will help us keep the same styles in Stencil-Select component without duplicating height, paddings, etc.

  Todos:

• [ ] +1
• [ ] +1

  Feedback:

none so far

[jeffkamo]

I wonder if we should be using variables in vellum?

It make sense for us to have variables in stencil because you don't edit those files directly – meaning we override things by redefining variables in _variable.scss. But we don't do that when making changes to vellum, since it's intentionally suppose to be changed directly in the vellum files.

[nastiatikk]

They are changing directly in the vellum files, even being the variables

P.S. From what I can see we already use them. Borders, colours, font-size, font-family, etc…

[nastiatikk]

The reason to specify them with $forms__something is to be able to use them in the stencil components instead of writing the same parameters in Vellum and in Stencil. But parameters itself set up in the Vellum.

[kpeatt]

I'm with @nastiatikk. They totally belong here. One place to change things instead of 10 sounds pretty great to me.

[jeffkamo]

fair enough, although most of these variables are only used once.

Assuming they'll be used for things like stencils, these variables will need to be declared in _variable.scss so we can do...

my-stencil__border-color: $forms__border-color;

Or would we be okay with overriding these stencil variables in _form.scss?

[nastiatikk]

This is how I see its collaboration with Stencil-select

$select__height: if(variable-exists(forms__element-height), $forms__element-height, 50px) !default;
$select__background-color: if(variable-exists(forms__background-color), $forms__background-color, #fff) !default;
etc.

I don't share the idea of putting all (ALL) variables in variables.scss. For me this file is more about global variables. As styling forms become something more local. But I see the conflict here.

[kpeatt]

@jeffkamo Why do they need to live in variables.scss?

[ry5n]

I think most of these represent premature abstraction. I’m with @jeffkamo. If your design is going to need to use the same value in many places, sure, make it a variable. But let’t not overwhelm folks with so many variables off the bat.

[jeffkamo]

That's where I thought stencil "settings" would be overridden. But with @nastiatikk's example above, that makes sense. Means no overriding actually needs to happen since it's handled by the stencil component instead.

[nastiatikk]

As we end up with a lot of variables (that are required to keep the same styles for all form elements) should we consider the idea of having Stencil-form component where we can keep all form elements (input, selects, textarea) as their performance is interdependent? From this point it looks like input is the same component as select.

[nastiatikk]

Only those CSS properties that were duplicated in c-select were put to variables @ry5n

[kpeatt]

input shouldn't need a component unless we want to attach things like icons to it... so actually maybe it does.

That said, should padding here be using $v-space and $h-space? Not that I love those variables but seems like they're related.

[ry5n]

In the last UI dev meeting, we decided that we would try to rely on a set of sane defaults (like our Vellum variables) for our components where possible, such as for minimum heights, padding, borders and do on.

So, here’s what I’d like to do: we need a designer to produce a styleguide for New Stencil™ from which we can derive our small set of sane defaults. Vellum may need changes to match, but we should do that in a new PR. We’ll also need to adjust the config vars in mobify/stencil-select#1. Until those are done, I move to close this PR.

@nastiatikk @avelinet @jeffkamo @kpeatt what do you think?

[avelinet]

Makes sense to me @ry5n, I think those sane defaults are definitely needed and will help with a number of other conversations we have been having around all this.

[nastiatikk]

Yes, I agree with you @ry5n Need to remember to update Stencil-select

[jeffkamo]

@ry5n Agreed, please proceed :)

[ry5n]

OK, thanks all. We have a JIRA ticket (CSOPS-1256) to track the creation of the variables we want for Stencil. That will inform changes to Vellum in which case we can open a new issue here.