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 4 UI components are not customizable enough, compiled list #17166

Closed kevryan2 closed 4 years ago

kevryan2 commented 5 years ago

Feature Request

Ionic version:

[x] 4.x

Current Behavior Ion-toggle only has 4 customizable Css properties, meaning that the background colors of the knob and the toggle (selected and unselected) are the only supported properties for editing.

Describe Preferred Solution In past versions, with the components not in the shadow DOM, they were highly customizable to provide great and easily editable UI. For example, placing custom border-radius on both the knob and the toggle are no longer possible. The same can also be said for components such as ion-radio. Because so much has been lost with this change, it would be nice if most (or at least more than 4 in the case of ion-toggle) CSS properties were exposed and could be set via variables.

Example of preferred solution given below:

Related Code ion-toggle { --handle-border-radius: 1px; --toggle-border-radius: 1px; }

Additional Context Would be great if the editable CSS properties for all UI elements could (at least) reach the level of ion-range.

manucorporat commented 5 years ago

Would you be up to submit a PR adding the css variables you expect?

kevryan2 commented 5 years ago

@manucorporat i could probably fork and work on a PR when I get a chance (hopefully in the next week or so) or I could map out and post the variables and their functions for the components that probably should be extended if you'd rather, not sure which you'd prefer?

manucorporat commented 5 years ago

yeah! both ways would be great ways to contribute! let's focus in adding the CSS variables you think are the most important ones. it's better to start small, we can always add new variables, but once it's there, we can't remove it, it becomes part of the public api

kevryan2 commented 5 years ago

@manucorporat Right now these are the variables I think should probably be exposed, I kept it small here as you suggested and modeled them after what you've exposed for ion-range. Like I said I can work on a PR soon, but I will not be back on my machine with the Ionic4 environment for about 10 days, so let me know if you'd still like me to issue one or you'll just include these edits in an upcoming rc release

  1. card --box-shadow --border-radius
  2. footer --background-color
  3. header --background-color
  4. radio --border-radius
  5. toggle --border-radius --handle-border-radius
  6. badge --border-radius
  7. content --background-image
kevryan2 commented 5 years ago

I have pulled in and compiled other bugs that have been reported and confirmed with UI components for the sake of having one central list.

Bug Fixes

  1. toggle-icon: fix background-color (described in notes below) in toggle.ios.scss line 49 .toggle-icon{ background-color: var(--background); } (like it is in line 47 toggle.md.scss .toggle-icon)

  2. ion-button ( #17042 ) :host(.focused) .button-native { background: var(--background-focused); color: var(--color-focused); }

Additional Requests

  1. ion-checkbox - stroke-width ( #16803 ) stroke-width: --stroke-width;

Notes For the unchecked toggle, you'll notice that there is a 1px "edge" on the unchecked toggle on iOS, where if you change the --background property the default background color still shows around the edge, I believe the solution would be number 1 below, and

lesrpo commented 5 years ago

what about the 'select-icon' for the ion-select? I don't find the way of change the color through ionic variables

kevryan2 commented 5 years ago

@manucorporat I've updated this issue to compile a couple other open issues regarding further specific instances. I'm still open to making a PR in a week when I'm able, but can you please explain how I would run my application with a local version of ionic so that I could test my updates before issuing the PR? Thanks! @lesrpo can you show which property can be exposed to fix this and I'll update the list?

chrisgriffith commented 5 years ago

I would add border radius of the ion-badge to this list as well

kevryan2 commented 5 years ago

@chrisgriffith thanks, good call I have added that... I will be looking into issuing a PR soon for this

brandyscarney commented 5 years ago

Thank you for the issue! Just now read through this, but I think the problem is actually with the precedence of styles. In general if the component can be styled using pure CSS we see no need for adding a variable. For example, the card component uses pure CSS on the host element to apply box-shadow and border-radius, so the following should work:

ion-card {
  box-shadow: 6px 1px 14px 14px red;
  border-radius: 25px;
}

However, because the card is scoped the Ionic styles are taking priority, and user styles need !important to override Ionic's.

Badge doesn't have this problem though, it works by setting border-radius directly:

ion-badge {
  height: 55px;
  width: 55px;
  border-radius: 15px;
}

I'm not sure what the goal is for the background of header/footer, but this can also be set with CSS. For content - it accepts the --background shorthand. Could you explain why the background image is also needed? Thank you!

Here's a codepen I threw together to hopefully help explain what I mean: https://codepen.io/brandyscarney/pen/jdLoYw

cc @manucorporat @liamdebeasi

brandyscarney commented 5 years ago

I'm still open to making a PR in a week when I'm able, but can you please explain how I would run my application with a local version of ionic so that I could test my updates before issuing the PR?

You should be able to find everything you need in our contributing documentation here: https://github.com/ionic-team/ionic/blob/master/.github/CONTRIBUTING.md#creating-a-pull-request

Please let me know if you run into problems as we want to make sure this stays up to date. 🙂

kevryan2 commented 5 years ago

@brandyscarney thanks for the reply, and thanks for setting up that codepen! I definitely see what you mean with certain components, especially the ones you listed and created the codepen for. I tested the other components in that codepen, and from what I can see: ion-toggle, or ion-radio are not customizable even using an !important tag. I THINK the reason is that I'm not able to select for a child element within the shadow DOM of an ionic component. (Ex. I can't seem to change the border-radius property for either .toggle-icon or .toggle-inner for the ion-toggle, same goes for .radio-icon of ion-radio).. all that being said here's the updated list that I believe still needs to be added/updated in a PR:

[REQUESTS]

  1. ion-toggle --border-radius --handle-border-radius
  2. ion-radio --border-radius
  3. ion-select --icon-color (as requested by @lesrpo , it seems you can only change the color of both the text and the icon together, I can't seem to change the color for just the icon)

[BUG FIXES]

  1. ion-button ( #17042 ) :host(.focused) .button-native { background: var(--background-focused); color: var(--color-focused); }
  2. toggle-icon: fix background-color (described in notes below) in toggle.ios.scss line 49 .toggle-icon{ background-color: var(--background); }

Notes For the unchecked toggle, you'll notice that there is a 1px "edge" on the unchecked toggle on iOS, where if you change the "--background" property the default background color still shows around the edge, I believe the solution would be number 1 above

brandyscarney commented 5 years ago

The list seems right, but the select icon / placeholder will be tricky to implement. I looked into getting it to work when we changed select to work automatically inside of a toolbar / item / any component with a color and I had trouble finding a solution that would work for all use cases since it needs to use currentColor in order to get the item/toolbar font color.

kevryan2 commented 5 years ago

@brandyscarney okay, I could see how that would be an issue but it's definitely a limitation (albeit a small one). Will you be looking into numbers 1 & 2 of the [REQUESTS] and [BUGS] for a future PR or should I still add issuing a PR to my task list (thanks for the link on how to do that by the way)?

brandyscarney commented 5 years ago

@KevRyan2 If you'd like to look into we appreciate all help! I usually throw examples of using custom styles into the standalone test of the component, unless that is overloaded then I'll make a custom test: https://github.com/ionic-team/ionic/blob/master/core/src/components/toggle/test/standalone/index.html

For the bugs it's easier to track if each of them are reported in separate issues, then we can look into them individually. I see button has one, so maybe just a separate issue for the toggle one?

troyanskiy commented 5 years ago

Hi @all I have started refactoring my app from ionic v3 to v4 and since I have a lot of ionic components modifications in the css deeply so will need to expose tons of variables. So I have created a directive in order to make css injections into the shadow DOM components and I would like to share it with you https://www.npmjs.com/package/ngx-shadow-class Feel free to create any type of issues in the repo.

barocsi commented 5 years ago

Where can I find a comprehensive list of theming options that were existing in V3 for example where are these?

$list-text-color: color($colors, list-text-color); $list-background-color: color($colors, list-background-color); $list-border-color: color($colors, list-border-color); $list-md-border-color: color($colors, list-border-color); $list-ios-border-color: color($colors, list-border-color); $item-divider-background: color($colors, list-divider-color); $item-md-divider-background: color($colors, list-divider-color); $item-ios-divider-background: color($colors, list-divider-color); $list-ios-activated-background-color: color($colors, list-activated-color); $list-md-activated-background-color: color($colors, list-activated-color);

and so on

chrisgriffith commented 5 years ago

Adding another variable request: The Card's margin values

brandyscarney commented 5 years ago

@barocsi There currently isn't a comprehensive list, however it is recommended now to just use CSS and change the element itself where you would previously change a Sass variable. For example, list text would change by using:

ion-list {
  color: color($colors, list-text-color);
}

As for item and item divider, there are some CSS variables available:

If the component has CSS variables they will be listed in the CSS Custom Properties section of their API doc. It is still on our TODO list to improve the customization documentation for components.

brandyscarney commented 5 years ago

@chrisgriffith Card doesn't need a CSS variable for margin as the margin property is on the host element. If adjusting the margin is not working it is likely you are running into this problem: https://github.com/ionic-team/ionic/issues/17425

Since scoped component's styles are taking higher priority over user's styles, you have to add !important to the style for it to take precedence, see this Codepen: https://codepen.io/brandyscarney/pen/ywvjGO?editors=1100

barocsi commented 5 years ago

I think the github workflow for web components is a general flaw for such projects. The components are buggy and need a lot of customization, not only for a given context but due to the high number of use cases. There will be so many requests by the community that exposing everything through CSS properties will be impossible. Hence developers will fork, compile and use a web component for themselves in an isolated manner, so they will not push it back to the community due to the uniqueness of the case. Double loss.

I think the workflow of web components will need to change, githubs workflow is just not suitable for it.

Just imagine how many (almost ALL of The) times did you need to override the CSS of a component? The amount of variations will make the exposes css variables to converge to the number of CSS properties, because I am pretty sure on the long run someone will request it to be exposed.

troyanskiy commented 5 years ago

I’m totally agree with @barocsi I would add also that even if you will add some new css variable and make a PR it will take monthes to merge the PR and make a new release of the FW. That’s why I’ve created dirty hack of shadow component to inject and overwrite CSS (npm ngx-shadow-class).

brandyscarney commented 5 years ago

Thanks for the feedback! We're working with what we have available now in terms of CSS features and support. We agree that the current state of CSS could use some improvement, and improvements are already underway. Chrome 73 actually just shipped with the :part feature which would solve a lot of these problems. See the spec on CSS parts here: https://www.w3.org/TR/css-shadow-parts-1/ and here's a blog that explains it a bit better: https://meowni.ca/posts/part-theme-explainer/

Basically, the :part feature will allow users to target the internal parts of shadow components without having to expose CSS variables for everything. However, until we find a way to polyfill the missing CSS features for unsupported browsers, we are using CSS variables and what we have available to allow customization. We're always open to suggestions and ideas and we are always actively looking to improve. We recently started working to use constructable stylesheets and have found that the API solves some big problems for us, such as:

We're doing our best to review issues and pull requests and merge what we can, but we have a small team and a lot of the time the pull request requires us to make or request changes which can cause a back and forth interaction before it can be merged. We love and appreciate the community's help reporting issues and creating pull requests though as it brings missing things to our attention and gives us a push to get them added in faster.

jonathanhines commented 5 years ago

Variable request: ion-select - opacity for .select-icon-inner --icon-opacity

Currently hard coded to a value of .33

galnova commented 5 years ago

Variable request: ion-select - opacity for .select-icon-inner --icon-opacity

Currently hard coded to a value of .33

I cannot change the opacity of the ion-icon in the menu page items. I've tried everything. It's currently at .25. I have tried controlling ion-item and it doesn't respond. I have even tried changing the --detail-icon-opacity and it doesn't change. I can add a custom class to the ion-item and that let's me control the color but it still wont give me control of the opacity of the right arrow. Please let me know when you can.

madhawa-se commented 5 years ago

this is really frustrating.ionic 3 was great. only thing we can do is ignore all ionic prebuilt component and use our own components.but then why do we even use ionic at the first place.just moved to flutter.

JoCedric commented 5 years ago

Hello,

I'm desperate to change the size of a toggle (width: 100%;). the handle is blocked. Does anyone have a solution?

rathodkaran07 commented 5 years ago

Hello,

Can any suggest how can I change ion-radio border radius to desired value?

liamdebeasi commented 4 years ago

Thanks for the issue! Most of the CSS Variables requested here have been added and will be available in an upcoming release of Ionic Framework.

For any additional requests please create new issues, separating each issue by component. For example, if you wanted to request 3 variables for ion-button and 2 for ion-toggle, please create one issue for the ion-button requests and one issue for the ion-toggle requests. This make it much easier for us to keep track of :slightly_smiling_face:.

The following list shows the status of the variables requested:

:white_check_mark: - Added to Ionic Framework :information_source: - CSS variable not needed. See table cell for details.

Component Variable Status
ion-badge --border-radius
ion-card --box-shadow ℹ️ Box shadow is set on the host element, so a CSS variable is not needed.
ion-card --border-radius ℹ️ Border radius is set on the host element, so a CSS variable is not needed.
ion-card --margin ℹ️ Margin is set on the host element, so a CSS variable is not needed.
ion-checkbox --stroke-width ✅ Added as --checkmark-width.
ion-content --background-image ℹ️ ion-content already provides a
--background variable that can be used to achieve the same result.
ion-footer --background-color ℹ️ Can be set directly on the element with CSS.
ion-header --background-color ℹ️ Can be set directly on the element with CSS.
ion-radio --border-radius ✅ Also added --inner-border-radius for customizing the checked indicator.
ion-toggle --border-radius
ion-toggle --handle-border-radius
ion-select --icon-color ✅ Resolved via CSS Shadow Parts.
ion-select --opacity ✅ Resolved via CSS Shadow Parts.
ion-select --icon-opacity ✅ Resolved via CSS Shadow Parts.
ionitron-bot[bot] commented 4 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.