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

md-outlined-select issues: empty menu, wrong colors, wrong size #4312

Closed
yegor-pelykh opened this issue May 18, 2023 · 17 comments
Closed

md-outlined-select issues: empty menu, wrong colors, wrong size #4312

yegor-pelykh opened this issue May 18, 2023 · 17 comments

Comments

@yegor-pelykh
Copy link

yegor-pelykh commented May 18, 2023

I noticed two issues with md-outlined-select:

  1. Even in the simplest use case of `md-outlined-select, the popup menu appears empty.

Screenshot

Code:

<md-outlined-select label="Select value">
    <md-select-option value="value1" headline="Value1"></md-select-option>
    <md-select-option value="value2" headline="Value2"></md-select-option>
</md-outlined-select>

Online Demo

  1. When overriding theme colors, the md-outlined-select popup menu colors do not change, but remain default.
@yegor-pelykh yegor-pelykh changed the title md-outline-select menu is empty md-outline-select issues: empty menu, wrong colors May 18, 2023
@christophe-g
Copy link
Contributor

index.ts:

import '@material/web/select/outlined-select.js';
import '@material/web/select/select-option.js';

@asyncLiz
Copy link
Collaborator

Yup! Need to import <md-select-option> as well :)

@asyncLiz asyncLiz closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
@asyncLiz
Copy link
Collaborator

asyncLiz commented May 18, 2023

If you're still having issues with the 2nd theming problem, can you add a repro?

@yegor-pelykh
Copy link
Author

yegor-pelykh commented May 19, 2023

Thank you for the info how to fix the 1st issue!

Hi @asyncLiz
Yes, the 2nd issue is still relevant.

Please look on this sample project. It uses custom theme from Material Theme Builder.
I also added some other controls besides the md-outlined-select.

https://lit.dev/playground/#gist=4ff91f11438699bb9adbf124a0d041e4

@yegor-pelykh
Copy link
Author

yegor-pelykh commented May 19, 2023

When use system dark theme, for example:

Screenshot

@yegor-pelykh
Copy link
Author

yegor-pelykh commented May 19, 2023

And an additional problem that I already saw in the second project.

  1. As you can see, the vertical flex layout is used on the body. All elements are stretched to full width, which is correct. But this does not apply to md-outlined-select. It takes up little space. And also the drop-down menu takes up little space.

@yegor-pelykh yegor-pelykh changed the title md-outline-select issues: empty menu, wrong colors md-outlined-select issues: empty menu, wrong colors, wrong size May 19, 2023
@yegor-pelykh
Copy link
Author

Hi @asyncLiz,
so could you please have a look on issues 2 and 3?

@asyncLiz
Copy link
Collaborator

For the colors, it looks like you're missing some of the new tokens, specifically the --md-sys-color-surface-container-* tokens, which menu uses. Check out the theming docs for a full list.

For the width, @e111077 can you take a look?

@asyncLiz asyncLiz reopened this May 22, 2023
@e111077
Copy link
Contributor

e111077 commented May 22, 2023

If the tokens are coming from the material theme builder, that project is out of date and that team is currently undergoing a rewrite of that project partly because it is not using the latest tokens; you need to add the surface-container tokens.

Here's a quick tool I scratched up to generate the sys level tokens with the latest set of color tokens

https://lit.dev/playground/#gist=abcc77f636c1b76506df860db6dc0faf&view-mode=preview

@e111077
Copy link
Contributor

e111077 commented May 22, 2023

Working on a fix to set width on the container, easy enough.

Am running into a problem, though, where we would want the menu to be to be the same width as the textbox where we can't make the headline text text-overflow: ellipsis because we do not know the widths of the start and end.

For example, we can set max-width on the select and have that cascade down to the menu and menu-item, but once we get to the menu-item we run into the problem where we have a flexbox with a width set on it, but the only flex-grow section (the headline text) does not know the width of the start and end, and thus cannot set its max size to allow overflowing.

In order to enable this, we'd have to require the user to tell md-select-option what is currently being slotted in which is bad DX. e.g.

<md-select-option has-start="icon" headline=${reallyLongText}>
  <md-icon data-variant="icon" slot="start">star</md-icon>
</md-select-option>

<md-select-option has-start="video-large" headline=${reallyLongText}>
  <video data-variant="video-large" slot="start" src="..." ...></video>
</md-select-option>

Here is an illustration of the problem:

┌────────────────────────────┐
│                            │
│                            │
│                            │
└────────────────────────────┘

┌────────────────────────────┐
│                            │
│ ┌────────────────────────┐ │
│ │                        │ │
│ │  ┌─┐ ┌───────────┐ ┌─┐ │ │
│ │  └─┘ └───────────┘ └─┘ │ │
│ │                        │ │
│ └────────────────────────┘ │
│                            │
│ ┌────────────────────────┐ │
│ │                        │ │
│ │  ┌─┐ ┌─────────────────┼─┼────────┐ ┌─┐
│ │  └─┘ └─────────────────┼─┼────────┘ └─┘
│ │                        │ │
│ └────────────────────────┘ │
│                            │
└────────────────────────────┘



width: 100%; display: flex;
┌────────────────────────┐
│                        │

┌────────────────────────┐
│                        │
│  ┌─┐ ┌─────────────────┼──────────┐ ┌─┐
│  └─┘ └──────▲──────────┼──────────┘ └─┘
│             │          │
└─────────────┼──────────┘
              │
              │
flex-grow:1; overflow:hidden;
text-overflow:ellipsis; word-wrap:nowrap;


  ┌────────────────────────┐
  │                        │
  │  ┌─┐ ┌─────────────────┼───┐ ┌─┐
  │  └─┘ └──────▲──────────┼───┘ └─┘
  │             │          │
  └─────────────┼──────────┘
                │
                │
      (max-)width: 100%;

Requires something like max-width: calc(100% - inline size of start - inline size of end)

copybara-service bot pushed a commit that referenced this issue May 23, 2023
related: #4312

This PR makes select inherit it's min. max, and normal width from host. It also makes list item make the headline not wrap. The effect this has is that the width of the menu in select will match that of the select input, but if the contents of the menu are wider than the width of the select, then the menu will be wider than the select input.

PiperOrigin-RevId: 534244900
@yegor-pelykh
Copy link
Author

yegor-pelykh commented May 23, 2023

If the tokens are coming from the material theme builder, that project is out of date and that team is currently undergoing a rewrite of that project partly because it is not using the latest tokens; you need to add the surface-container tokens.

Here's a quick tool I scratched up to generate the sys level tokens with the latest set of color tokens

https://lit.dev/playground/#gist=abcc77f636c1b76506df860db6dc0faf

Looks very interesting and cool! But could you advice me what should be used as the "Source color" in my case? I set the following parameters on the Material Theme Builder for the theme that I need:

  • primary: #F67A00
  • secondary: #00A8E2
  • tertiary: #3BB174
  • neutral: #171003

Which one to use to generate correct --md-sys-color-surface-container-* tokens?

@e111077
Copy link
Contributor

e111077 commented May 24, 2023

updated the link above. Please use the Dynamic color scheme

@yegor-pelykh
Copy link
Author

I've added all the missing tokens that are shown in your project but were missing from the Material Theme Builder tokens:

  /* light, additional colors */
  --md-sys-color-highest-surface-light: #e8d8be;
  --md-sys-color-surface-dim-light: #e8d8be;
  --md-sys-color-surface-bright-light: #fff8f2;
  --md-sys-color-surface-container-lowest-light: #ffffff;
  --md-sys-color-surface-container-low-light: #fff2df;
  --md-sys-color-surface-container-light: #fcecd1;
  --md-sys-color-surface-container-high-light: #f7e6cc;
  --md-sys-color-surface-container-highest-light: #f1e0c6;
  --md-sys-color-inverse-on-primary-light: #572700;
  /* dark, additional colors */
  --md-sys-color-highest-surface-dark: #413826;
  --md-sys-color-surface-dim-dark: #191204;
  --md-sys-color-surface-bright-dark: #413826;
  --md-sys-color-surface-container-lowest-dark: #140d02;
  --md-sys-color-surface-container-low-dark: #221a0a;
  --md-sys-color-surface-container-dark: #271e0e;
  --md-sys-color-surface-container-high-dark: #322918;
  --md-sys-color-surface-container-highest-dark: #3d3421;
  --md-sys-color-inverse-on-primary-dark: #ffe8dc;

Then, I added the missing variables to the light and dark theme that refer to the new tokens:

light:

 --md-sys-color-highest-surface: var(--md-sys-color-highest-surface-light);
 --md-sys-color-surface-dim: var(--md-sys-color-surface-dim-light);
 --md-sys-color-surface-bright: var(--md-sys-color-surface-bright-light);
 --md-sys-color-surface-container-lowest: var(--md-sys-color-surface-container-lowest-light);
 --md-sys-color-surface-container-low: var(--md-sys-color-surface-container-low-light);
 --md-sys-color-surface-container: var(--md-sys-color-surface-container-light);
 --md-sys-color-surface-container-high: var(--md-sys-color-surface-container-high-light);
 --md-sys-color-surface-container-highest: var(--md-sys-color-surface-container-highest-light);
 --md-sys-color-inverse-on-primary: var(--md-sys-color-inverse-on-primary-light);

dark:

 --md-sys-color-highest-surface: var(--md-sys-color-highest-surface-dark);
 --md-sys-color-surface-dim: var(--md-sys-color-surface-dim-dark);
 --md-sys-color-surface-bright: var(--md-sys-color-surface-bright-dark);
 --md-sys-color-surface-container-lowest: var(--md-sys-color-surface-container-lowest-dark);
 --md-sys-color-surface-container-low: var(--md-sys-color-surface-container-low-dark);
 --md-sys-color-surface-container: var(--md-sys-color-surface-container-dark);
 --md-sys-color-surface-container-high: var(--md-sys-color-surface-container-high-dark);
 --md-sys-color-surface-container-highest: var(--md-sys-color-surface-container-highest-dark);
 --md-sys-color-inverse-on-primary: var(--md-sys-color-inverse-on-primary-dark);

And it works!
Confirming that the md-outlined-select element now has the correct menu colors.

Lit playground:
https://lit.dev/playground/#gist=16a7863e1a9b090b098d5b5d34cb9cc5

@yegor-pelykh
Copy link
Author

Could you think of any solution for the correct width of the md-outlined-select, as well as the correct width of the menu so that its width matches the width of the element itself?

@e111077
Copy link
Contributor

e111077 commented May 25, 2023

As a compromise for now, I'm shooting for this behavior with min-width 100% since we can't ellipse truncate rn:

min-width-select.mp4

copybara-service bot pushed a commit that referenced this issue May 26, 2023
related: #4312

This PR makes select inherit it's min. max, and normal width from host. It also makes list item make the headline not wrap. The effect this has is that the width of the menu in select will match that of the select input, but if the contents of the menu are wider than the width of the select, then the menu will be wider than the select input.

PiperOrigin-RevId: 534244900
copybara-service bot pushed a commit that referenced this issue May 26, 2023
related: #4312

This PR makes select inherit it's min. max, and normal width from host. It also makes list item make the headline not wrap. The effect this has is that the width of the menu in select will match that of the select input, but if the contents of the menu are wider than the width of the select, then the menu will be wider than the select input.

PiperOrigin-RevId: 535695646
@e111077
Copy link
Contributor

e111077 commented May 27, 2023

The above change has been submitted

@asyncLiz
Copy link
Collaborator

asyncLiz commented Aug 2, 2023

I think this is fixed now, including exposing ::part(menu) for customizing the size

@asyncLiz asyncLiz closed this as completed Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants