Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match Lumo Styling #6

Closed
Juchar opened this issue Feb 26, 2019 · 3 comments · Fixed by #7
Closed

Match Lumo Styling #6

Juchar opened this issue Feb 26, 2019 · 3 comments · Fixed by #7
Assignees
Labels
enhancement New feature or request

Comments

@Juchar
Copy link

Juchar commented Feb 26, 2019

The current implementation does not match the styling of the rest of the lumo components.
If using it in a form this is clearly a visual break.

It would be nice to have a theme that matches lumo (at least it should have the same style as the default combo box).

One example can be found here:
https://github.com/vaadin/vaadin-combo-box/issues/88#issuecomment-384958301

@gatanaso gatanaso added the enhancement New feature or request label Feb 26, 2019
@gatanaso
Copy link
Owner

Hi @Juchar,

Thank you for the feedback and the suggestion. This is something that I have been considering and others have mentioned to me as well :)
I have already started to work on the theming support for this component and will be updating the styles to match the lumo theme as closely as possible.

BR,
Goran

@gatanaso gatanaso self-assigned this Feb 27, 2019
@Juchar
Copy link
Author

Juchar commented Mar 1, 2019

Thank you already, great work!

I have seen that you use the following code to generate a border:

border: 1px solid;
border-radius: calc(var(--lumo-border-radius) / 2);
border-color: var(--lumo-contrast-10pct);
/* Omitted the rest for clarity */

Which produces the following result:
image

May I ask why not just using:

/* border: 1px solid; */
border-radius: var(--lumo-border-radius);
/* border-color: var(--lumo-contrast-10pct); */
/* Omitted the rest for clarity */

which produces the following output:
image

This would better match the default style of ComboBox & TextField:

image
image

Also I would slightly change the style of the tokens (completely subjective, personal opinion. I assume I can style it later on myself using a theme-for if you think it's a bad idea):

Now:
image

After:
image

Using:

[part="token"] {   
    background-color: var(--lumo-contrast-10pct);
    padding-left: calc(2*var(--lumo-space-xs));
    border-radius: var(--lumo-border-radius);
    /* Omitted the rest for clarity */
}

[part="token-label"] {
    /* Omitted the rest for clarity */
    /* padding-right: var(--lumo-space-xs); */
}

What do you think about it?

@gatanaso
Copy link
Owner

gatanaso commented Mar 2, 2019

Hi @Juchar ,

Thank you for the feedback and for the improvement ideas!

...
Which produces the following result:
image
...
which produces the following output:
image
...
This would better match the default style of ComboBox & TextField:
...

Yes, those would be much better defaults and they align nicely with the existing components 👍

Also I would slightly change the style of the tokens (completely subjective, personal opinion. I assume I can style it later on myself using a theme-for if you think it's a bad idea):

Now:
image
...
After:
image

What do you think about it?

I think that this is also a great improvement suggestion and it makes the tokens look much better 👍

However I would continue to use a background-color value of --lumo-contrast-20pct as it's a very small difference, but it makes the token chips stand out just a bit more (completely subjective, personal opinion here as well :) ).

Thank you again for providing these improvement ideas, I really appreciate it and will update the styles.

P.S. pull requests are always welcome! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants