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-button attaches class "md-undefined" to host element when on color provided #75

Closed
traviskaufman opened this issue Feb 12, 2016 · 14 comments
Milestone

Comments

@traviskaufman
Copy link
Contributor

Given an html snippet with an md-button directive like so:

<button md-button>flat</button>

This produces the following ouput:

<button md-button ... class="md-undefined">...</button>

What it should produce is:

<button md-button ...>...</button>

The md-undefined class is getting attached due to this code since setClassList() returns "md-undefined" when the color attribute is unset.

I tried working around the by returning an empty string when color is undefined, but that resulted in this html:

<button md-button ... class>...</button>

Which is also invalid.

@beaverusiv
Copy link

Maybe it should have a default;
setClassList() {return this.colour?md-${this.color}:'md-white';}

@sothychan
Copy link

My thought is to not include the class attribute if color is undefined, but I'm not sure how to modify the "host" metadata property in the component. Still learning angular2 to see how to get the component to not render with the class attribute if color is undefined. Do you guys know how?

@beaverusiv
Copy link

If returning an empty string still adds 'class' it could be an issue with Angular itself.

@zoechi
Copy link

zoechi commented Feb 16, 2016

If both

 setClassList() {return this.color ? `md-${this.color}` : '';}

and

 setClassList() {return this.color ? `md-${this.color}` : null;}

result in

<button md-button ... class>...</button>

I'd consider this an Angular bug.

@bakgat
Copy link

bakgat commented Feb 17, 2016

I've made a workaround for this. I'm not sure if it's good practice.

Instead binding to the class property, I'm injecting ElementRef into the constructor.
Then I let the component listen to Input() with getter and setter for color.

    color_: string;
    @Input()
    set color(value) {
        this.color_ = value;
        if (this.elementRef !== undefined) {
            this.elementRef.nativeElement.classList.add('md-' + this.color_);
        }
    }

    get color(): string {
        return this.color_;
    }

    constructor(public elementRef: ElementRef) {}

This way no class attribute is rendered on the DOM, when there's no color property provided.

What do you think?

@bakgat
Copy link

bakgat commented Feb 17, 2016

I even noticed now that when adding an extra class to a md-button, this class is overriden without the fix.

ie:

    <button md-button class="hamburger-button"></button> 
OR
    <button md-button class="hamburger-button" color="accent"></button>

results both in
<button md-button class="md-undefined"></button>

expected:

    <button md-button class="hamburger-button"></button>

    <button md-button class="hamburger-button md-accent"></button>

@zoechi
Copy link

zoechi commented Feb 18, 2016

AFAIK using nativeElement this way is discouraged

this.elementRef.nativeElement.classList.add('md-' + this.color_);
    }

as it prevents the code to run in webworker

@zoechi
Copy link

zoechi commented Feb 18, 2016

If binding to class doesn't work as expected I'd use ngClass instead.

@bakgat
Copy link

bakgat commented Feb 18, 2016

@zoechi
Can you show how? I can't get ngClass to work on 'host'.

@zoechi
Copy link

zoechi commented Feb 18, 2016

Sorry, forgot this is about host. ngClass can't be used for this.

@bakgat
Copy link

bakgat commented Feb 18, 2016

So, what are the other options to prevent the problems stated above?

@zoechi
Copy link

zoechi commented Feb 18, 2016

I'd create a bug report for the behavior where class is left standing when an empty string is returned.

@bakgat
Copy link

bakgat commented Feb 19, 2016

Thanks to @zoechi at Stack Overflow I believe we've found a solution for two issues related to this one.

  1. <button md-button></button> results in <button md-button class="md-undefined"></button>
  2. <button md-button color="accent" class="myCustomClass"></button> results in ```

Drop [class] on host and add following code might be a solution:

    private color_: string;

    // @see http://stackoverflow.com/questions/35506395/looking-for-nativeelement-classlist-add-alternative/35514006
    @Input()
    set color(value: string) {
        this.color_ = value;

        this.renderer.setElementClass(this.elementRef, 'md-' + this.color_, true);
    }

    get color(): string {
        return this.color_;
    }

    constructor(private elementRef: ElementRef, private renderer: Renderer) {
    }

@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 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants