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

[Ionic v4] override the opacity of ion-button if disabled #16965

Closed
Nazirovich opened this issue Jan 5, 2019 · 15 comments

Comments

@Nazirovich
Copy link

commented Jan 5, 2019

Bug Report

Ionic version:

[x] 4.x

Expected behavior:

The 'disabled' rules (opacity: 1) should be applied.

Steps to reproduce:

Steps to reproduce the behavior:

  1. Add disabled attribute to ion-button
  2. Style it via pseudo or attribute selector of disabled
  3. Css rules are not applied

Related code:

 <ion-button color="success" type="submit" [disabled]="true">
        LOG IN
 </ion-button>

When an ion-button is disabled, the following styles is used

.button-native[disabled] {
     cursor: default;
     opacity: .5;
     pointer-events: none;
}
/* Not applied */
ion-button[disabled],
ion-button[disabled="true"],
ion-button:disabled {
   --opacity: 1!important;
   opacity: 1!important;
}

/* Not applied */
.button-native[disabled] {
   --opacity: 1!important;
   opacity: 1!important;
}

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.6.0
   Ionic Framework               : @ionic/angular 4.0.0-rc.0
   @angular-devkit/build-angular : 0.11.4
   @angular-devkit/schematics    : 7.1.2
   @angular/cli                  : 7.1.0
   @ionic/angular-toolkit        : 1.2.0

Cordova:

   cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
   Cordova Platforms     : none
   Cordova Plugins       : not available

System:

   NodeJS : v8.11.3 (C:\Program Files\nodejs\node.exe)
   npm    : 6.3.0
   OS     : Windows 10

@ionitron-bot ionitron-bot bot added the triage label Jan 5, 2019

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Jan 15, 2019

Thanks for your issue! Your's is not working because opacity is set at the .button-native and there is no access to it from outside.

We have to add a new css custom property, e.g. --opacity. then you can use

ion-button.button-disabled {
   --opacity: 1;
}

So it's a feature request :)

@giantpeach

This comment has been minimized.

Copy link

commented Jan 17, 2019

The ability to style a button is a basic requirement. Moving from v3 to v4, styling has become a lot more difficult.

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Jan 17, 2019

Ionic 4 is using web components and shadow DOM - of course this has benefits, but also disadvantages. But the benefits are higher. Styling components behind shadow dom is only possible with css custom properties and this is needed here :) Only the host element can be styled without it

@Floppy1009

This comment has been minimized.

Copy link

commented Jan 22, 2019

The opacity is missing in other elements as well. For instance I currently have the issue for ion-checkbox which I want to disable without changing the opacity (including the label).
Please add these css variables as well.
Is there another way to achieve this in the meantime? I tried to add click events and added "stopPropagation()" which didn't work unfortunately.
Any other idea?

@UntappedSolutions

This comment has been minimized.

Copy link

commented Feb 5, 2019

I am using this bit of code and the first 2 work but not opacity. Any ideas?
ion-button.button-disabled{ --background:var(--ion-color-primary); --color:var(--ion-color-primary-contrast); --opacity: 1; }

@troyanskiy

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2019

@jtth

This comment has been minimized.

Copy link

commented Mar 20, 2019

Yeah, this is really frustrating, and the opacity variable does not work to fix things.

@Levy4u

This comment has been minimized.

Copy link

commented Apr 26, 2019

I can't figure it out either. I have a bunch of forms that the admin fills out but want it disabled for users and not be hard to see. Right now i have to make a duplicate of every single form but make it a normal grid passing in the values to text fields.

@Kernolsie

This comment has been minimized.

Copy link

commented Apr 30, 2019

Ditto, the --opacity variable does not seem to have any effect on my disabled button

@darkguy2008

This comment has been minimized.

Copy link

commented May 27, 2019

So this issue is like a half a year old, any improvements? the whole shadow DOM thing is utter crap, it adds more headaches than it solves, because we're unable to solve things on our own without waiting for you guys to move your lazy butts and add the variable.

@rphiller

This comment has been minimized.

Copy link

commented May 28, 2019

Hey. there is a workaround.

Remove disabled Attribute and add

[style.pointerEvents]="'none'"

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

@darkguy2008 I'd like to kindly remind you that we do have a COC and it should be followed by everyone including those commenting on our GitHub issues: https://github.com/ionic-team/ionic/blob/master/CODE_OF_CONDUCT.md

I'm sorry you think we are being lazy by not adding CSS variables. We're pretty transparent about what we are working on. We update our project board every other week: https://github.com/ionic-team/ionic/projects/3. We also have been doing releases bi-weekly: https://ionicframework.com/docs/release-notes. If you'd ever like to see what each specific member of the team is working on, we update the following public Google doc with our framework meeting notes every Monday: https://docs.google.com/document/d/1LrPDUkfXpqPIsghaSCxHyN1GIZ0TK2bwFxOZx2GKWtI/edit?usp=sharing.

CSS variables and Shadow DOM solve a lot of problems actually. They greatly reduce the bundle size, improving your app's startup time. They remove the need to compile all of Ionic's CSS (thus improving build times) by getting rid of the Sass dependency at build time. They allow you to dynamically change your application's theme at runtime without having to create double the CSS. Components that use CSS variables are required to be Shadow DOM (or scoped).

The --opacity variable was actually added awhile ago, this issue just occurs because it isn't used in the case of a disabled button. The fix should be changing the following in button.scss:

:host(.button-disabled) {
  pointer-events: none;
}

:host(.button-disabled) .button-native {
  cursor: default;
  opacity: .5;
  pointer-events: none;
}

to

:host(.button-disabled) {
  --opacity: .5;

  pointer-events: none;
}

:host(.button-disabled) .button-native {
  cursor: default;

  pointer-events: none;
}

If anyone would like to help out with this, PRs force us to take a look at issues and get fixes in faster. Otherwise I will add this to my list to look at in the near future. Thanks!

@ionitron-bot

This comment has been minimized.

Copy link

commented Jun 10, 2019

This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.

If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.

For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.

Thank you!

@Ivnosing

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

@brandyscarney thanks for the reference to the issue. If it's OK, I will submit a PR tomorrow implementing this feature (my first one!).

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@Ivnosing That would be great, thank you! Could you please add an example that shows the custom button opacity to one of the tests too, maybe here: https://github.com/ionic-team/ionic/blob/master/core/src/components/button/test/basic/index.html#L84-L87? This will help our testing so that we avoid a future regression. 🙂

@brandyscarney brandyscarney added this to Backlog 🤖 in Ionic Core via automation Jun 11, 2019

Ionic Core automation moved this from Backlog 🤖 to Done 🎉 Jun 11, 2019

brandyscarney added a commit that referenced this issue Jun 11, 2019

fix(button): set opacity on the host for disabled button (#18509)
allows for customization of the disabled button opacity

fixes #16965
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.