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.02k stars 13.51k forks source link

feat: ion-searchbar default icon in IonicConfig #22946

Closed Spinnenzunge closed 3 years ago

Spinnenzunge commented 3 years ago

Feature Request

Expose IonicConfig property for ion-searchbar default icon for theming purposes in IonicModules root import.

Ionic version:

[x] 5.x

Describe the Feature Request

Provide ion-searchbar default icon properties (cancel-button, clear, search) in IonicConfig like it is done with e.g. backButtonIcon

Describe Preferred Solution

export interface IonicConfig {
  /**
   * Overrides the default icon in all `<ion-searchbar>` components.
   */
  searchBarIcon?: string;

  /**
   * Overrides the default clear icon in all `<ion-searchbar>` components.
   */
  searchBarClearIcon?: string;

  /**
   * Overrides the default cancel button icon in all `<ion-searchbar>` components.
   */
  searchBarCancelButtonIcon?: string;
}
liamdebeasi commented 3 years ago

Thanks for the issue. Can you provide a use case for this?

Spinnenzunge commented 3 years ago

Use case would be branding these components. For our app we want to use our companies style of components, that includes a searchbar with our custom icon set, in this case the magnifying glass and the close icon. Having to add this on every ion-searchbar in the template seems a little bit excessive, especially considering that exactly that feature already is provided for the ion-back-button.

EinfachHans commented 3 years ago

As soon as @liamdebeasi confirms this as a new feature for the Ionic Franework i can provide a PR 😊

Spinnenzunge commented 3 years ago

Thanks @EinfachHans. Btw. the same feature request could be made for the default detailIcon (drill down arrow) on the ion-item component.

liamdebeasi commented 3 years ago

It sounds like the goal here is to use your custom icons instead of the built-in Ionicons. Is that correct?

If so, then it is probably better to override the icon directly using the addIcons function exported from ionicons. Here is an example:

// This should be added to the main entry point in an app before any page is loaded

import { addIcons } from 'ionicons';

// This is the path to the icon file
const customIconPath = 'assets/icons/custom-search.svg';

addIcons({
  // Sharp is used on MD, outline is used on iOS
  'search-sharp': customIconPath,
  'search-outline': customIconPath
});

Using this approach, your custom icon is used whenever your app references the search icon. Does something like this work for your use case?

Spinnenzunge commented 3 years ago

Thanks for the reply @liamdebeasi

Yes exactly, the goal is to use custom icons instead of ionicons. I just checked weather or not it works and it does work indeed if I add following code to my app constructor:

addIcons({
  'search-sharp': 'assets/icon/search.svg',
  'search-outline': 'assets/icon/search.svg',
  'chevron-forward': 'assets/icon/right.svg',
  'close-circle': 'assets/icon/close.svg',
  'close-sharp': 'assets/icon/close.svg',
});

But is this really the correct approach? If I understand it correctly, as soon as I use the addIcons() method like that I overwrite the existing ionicon icons for that given name with my custom svgs. So far I used the src attribute on the ion-icon instead so I had not to touch the ionicon names at all.

And if that is the way to go, should then not the IonicConfig remove the other defaultIcon options as well? Right now there is kind of a mix for branding the default icons for Ionic components.

For my use case this is solved and this feature request can be closed but if I had an App that wanted to use the ionicons as well as custom icons, that would not solve my problem and maybe needs further discussion.

liamdebeasi commented 3 years ago

Yes, that would use your custom SVG all the time. Are there use cases where you would want to have two different styles of icons? To me, that would seem confusing but maybe there are some good use cases I am not thinking of.

The global IonicConfig icon options are really there for legacy purposes as the addIcons function did not exist for ionicons during the Ionic Framework v3 days as far as I am aware.

Spinnenzunge commented 3 years ago

Hmm considering use cases for using both: Probably not that many, at least not one I could think of right now.

From a developer experience point of view it just feels wrong to overwrite a couple of ionicon names and their nuances (sharp, outline etc.) with branded icons and leaving the rest of them as default. Realistically speaking we will never have all the icons and nuances in our brand that the ionicon provide.

That said, I can completely solve my problem with using addIcons() for branding the ion-searchbar and other icons.

liamdebeasi commented 3 years ago

It's probably worth exploring other avenues for using custom icons within the framework. Where we don't have a new use case, and addIcons solves the original use case, I am going to close this for now. If anyone runs into a scenario where revisiting this feels necessary please feel free to open a new issue. Thank you!

ionitron-bot[bot] commented 3 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.