ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.08k stars 13.51k forks source link

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

Closed Nazirovich closed 5 years ago

Nazirovich commented 5 years ago

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
paulstelzer commented 5 years ago

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 commented 5 years ago

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

paulstelzer commented 5 years ago

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

GerTea commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Hi @paulstelzer check that out https://www.npmjs.com/package/ngx-shadow-class

saralilyb commented 5 years ago

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

Levy4u commented 5 years ago

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 commented 5 years ago

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

darkguy2008 commented 5 years ago

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 commented 5 years ago

Hey. there is a workaround.

Remove disabled Attribute and add

[style.pointerEvents]="'none'"

brandyscarney commented 5 years ago

@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[bot] commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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. 🙂

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.