feat(toast): add icon property to show icon at start of toast content

[domske]

Pull request checklist

fixes #23524

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

The toast does not allow to display an icon.

What is the new behavior?

You can now optionally specify an icon to show with the toast message.

md

ios

Does this introduce a breaking change?

• Yes
• No

Other information

I added linter ignore blacklist property comment to allow padding and margin-left. Feel free to change it. I don't know what you prefer.

Should be available in Ionic 5.x

[EinfachHans]

Maybe better to use icon instead of name Property to allow custom icons?

[domske]

@EinfachHans

You can already use custom icons with the name attribute.
Just place your svg icons to the Ionic icons svg build folder and you can easily use both (build-in and custom icons) as name.

For example in Angular. You can automatically copy your custom icons with the build.

angular.json

projects {} > app {} > architect {} > build {} > options {} > assets []:

{
  "glob": "**/*.svg",
  "input": "src/assets/custom-icons",
  "output": "./svg"
}

Place e.g. an icon like custom-my-icon.svg to your src assets and you can use the ion-icon (or the toast icon prop) like:

<ion-icon name="custom-my-icon"></ion-icon>

We could replace name with the icon prop. But does it also work with my solution above? I have to test it... Because I use the name solution. It's nicer (names) than src-paths.

Icon Source

We probably should also complete the ion-icon documentation. Because I cannot find the icon prop.
Only in the source code. See docs.

[EinfachHans]

As far as i understand the icon prop could also take for example "assets/..." and determinates if its a asset icon or ionicon. Also the toast button icon uses the icon Property

[domske]

Ok I replaced the name property with icon. Not yet tested in productive app. But I think it will work since the button icon also uses this property. Tests passed.

[liamdebeasi]

Thanks for the PR! I have included comments regarding changes which are required in order for this feature to get merged.

Additionally, can you please add some usage examples in the usage directory? You will need to update the angular.md, javascript.md, react.md, stencil.md, and vue.md files, run npm run build, and then push that result.

I think we can add the icon example inside of the existing presentToastWithOptions function in these usage files.

[liamdebeasi]

Suggested change

	/* stylelint-disable property-blacklist */
	margin-left: 15px;
	padding: 10px;
	/* stylelint-enable property-blacklist */
	font-size: 1.4em;
	@include margin(0, 0, 0, 15px);
	@include padding(10px, 10px, 10px, 10px);

We need to include the mixins here to account for RTL mode. Currently, the styles are not consistent between LTR and RTL modes, but the mixins will handle this switch for us.

Also, no need to add the font size here as it should apply in the same block as .toast-button-icon (see my other comment)

[domske]

Ok, change requests pushed...

I just wonder:

• Why this.icon on the first line? It's a simple short path and equal to the the other properties like this.message and this.header. It's the same style. (Not yet changed.)

• Why lazy={false} and not lazy="false" since aria-hidden="true"? Ok, the type is different (boolean and string) but does this matter here?

Nothing important. I'm just wondering.

[liamdebeasi]

Sorry I had two different PRs open and was referencing the wrong one 🤦 . Ignore the this.icon comment.

The ariaHidden property expects a string: https://github.com/ionic-team/ionicons/blob/master/src/components/icon/icon.tsx#L38
But the lazy property expects a boolean: https://github.com/ionic-team/ionicons/blob/master/src/components/icon/icon.tsx#L82

Since the ariaHidden property eventually gets set as the aria-hidden attribute, we need to accept a string since the aria-hidden attribute cannot accept a boolean (it's just plain HTML at that point)

[domske]

Ok, please let me know if there are any pending change requests. All change requests were implemented. Or did I miss something?

[willmartian]

Looks good! Just a small docs change from me to reflect the switch from name to icon.

[liamdebeasi]

Great job! We are good to merge when the tests finish.

[liamdebeasi]

Merged. Thank you!