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

Some sass selector expressions don't work when nested in a theme class #4077

Closed
xmeng1 opened this issue Apr 13, 2017 · 5 comments · Fixed by #4145
Closed

Some sass selector expressions don't work when nested in a theme class #4077

xmeng1 opened this issue Apr 13, 2017 · 5 comments · Fixed by #4145
Labels
P2 The issue is important to a large percentage of users, with a workaround

Comments

@xmeng1
Copy link

xmeng1 commented Apr 13, 2017

Bug, feature request, or proposal:

I have searched this problem and find #2662, but when I try to use it I found, the select component still has a problem.

After setting the theme for the overlay container, the overlying theme of select is changed.

However the problem is the Placeholder and the underline, the color is still the previous theme


I try to find the reason of the issue and find that there are some CSS define for the placeholder and underline as follows:

material2-master/src/lib/select/_select-theme.scss

.mat-select-trigger {
    color: mat-color($foreground, hint-text);

    .mat-select:focus:not(.mat-select-disabled) & {
      color: mat-color($primary);
    }

    .mat-select:not(:focus).ng-invalid.ng-touched:not(.mat-select-disabled) & {
      color: mat-color($warn);
    }
  }

  .mat-select-underline {
    background-color: mat-color($foreground, divider);

    .mat-select:focus:not(.mat-select-disabled) & {
      background-color: mat-color($primary);
    }

    .mat-select:not(:focus).ng-invalid.ng-touched:not(.mat-select-disabled) & {
      background-color: mat-color($warn);
    }
  }

So when the Select is focused, these CSS Pseudo-Classes is activated, and the color is the previous theme. I think all DOM add a class of new theme .xxxxx (such as .red-indigo), but the pseudo-class is activated, this new theme class is not added.

I try to solve it, but I cannot find the related code to add class to the DOM

I make a demo and deploy on Firebase

https://github.com/xmeng1/ng2-material-admin

https://ng2-material-admin.firebaseapp.com/

What is the expected behavior?

What is the current behavior?

What are the steps to reproduce?

Providing a Plunker (or similar) is the best way to get the team to see your issue.
Plunker template: https://goo.gl/DlHd6U

What is the use-case or motivation for changing an existing behavior?

Which versions of Angular, Material, OS, browsers are affected?

Is there anything else we should know?

@xmeng1
Copy link
Author

xmeng1 commented Apr 13, 2017

By the way, I think the placeholder of input has the same problem which not display the primary color,

@xmeng1
Copy link
Author

xmeng1 commented Apr 13, 2017

I found another issue for the overlay if I put two Select in one component, and one is use system theme. the other one is put into the second theme. The expected behaviour is the first use the system theme, the second use the second theme. But I think current implementation, all the overlay will add the second class name.

@jelbourn
Copy link
Member

Took @kara a while to figure this out, but this is indeed a real bug.

The issue is that some styles are using the sass & symbol like this:

    .mat-select:focus:not(.mat-select-disabled) & {
      background-color: mat-color($primary);
    }

The problem is that & expands to the entire parent selector, which in this case includes the theme class, so that the output is

    .mat-select:focus:not(.mat-select-disabled) .red-indigo .mat-select-trigger {
      background-color: ...
    }

To fix this, theming files should never use the & symbol at the end of a selector expression.

@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Apr 13, 2017
@jelbourn jelbourn changed the title The multiple theme cannot apply to the Select component Some sass selector expressions don't work when nested in a theme class Apr 13, 2017
xmeng1 added a commit to xmeng1/ng2-material-admin that referenced this issue Apr 15, 2017
by using the newest method which fix the overlay bug.

but there is still has some problems:

1. input placeholder
2. select placeholder and underline
3. slide bar

ref issues:
[multiple themes don't work with overlay-based components](angular/components#2662)
[Some sass selector expressions don't work when nested in a theme class](angular/components#4077)
xmeng1 added a commit to xmeng1/ng2-material-admin that referenced this issue Apr 15, 2017
by using the newest method which fix the overlay bug.

but there is still has some problems:

1. input placeholder
2. select placeholder and underline
3. slide bar

ref issues:
[multiple themes don't work with overlay-based components](angular/components#2662)
[Some sass selector expressions don't work when nested in a theme class](angular/components#4077)
@xmeng1
Copy link
Author

xmeng1 commented Apr 16, 2017

@jelbourn you are right, I just replace the Ampersand to the normal format in _theming.scss, the multiple themes work well now. Maybe I could try to fix it later.

crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 18, 2017
* Fixes certain theme selectors being broken due to uses of the `&` operator at the end of the selector.
* Adds a custom Stylelint rule to catch future improper uses of the ampersand inside themes.

Relates to angular#3928.
Fixes angular#4077.
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 21, 2017
* Fixes certain theme selectors being broken due to uses of the `&` operator at the end of the selector.
* Adds a custom Stylelint rule to catch future improper uses of the ampersand inside themes.

Relates to angular#3928.
Fixes angular#4077.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants