Skip to content

feat(material/button): allow appearance to be set dynamically #30626

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

Merged
merged 3 commits into from
Mar 14, 2025

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 13, 2025

Adds the following features to the button:

  • Allows the appearance of a button to be set dynamically using the matButton input.
  • Aligns the terminology with the M3 spec. Currently the names are derived from an old spec.
  • Adds the ability to set the default appearance for buttons.
  • Adds a matIconButton selector to the icon button for consistency.
  • Adds a matFab selector to the FAB for consistency.
  • Adds a matMiniFab selector to the mini FAB for consistency.

All of these changes are backwards-compatible and allow us to evolve the button in the future.

BREAKING CHANGE:

  • ButtonVariant which is returned by MatButtonHarness.getVariant no longer includes the appearance of the button. Use MatButtonHarness.getAppearance instead.

Fixes #15367.
Fixes #29841.

@crisbeto crisbeto added target: major This PR is targeted for the next major release merge: preserve commits When the PR is merged, a rebase and merge should be performed labels Mar 13, 2025
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit area: material/button labels Mar 13, 2025
@crisbeto crisbeto force-pushed the 15367/dynamic-button-type branch from a8f588e to c6cb080 Compare March 13, 2025 10:37
@crisbeto crisbeto requested a review from andrewseguin March 13, 2025 10:37
@crisbeto crisbeto marked this pull request as ready for review March 13, 2025 10:37
@crisbeto crisbeto requested a review from a team as a code owner March 13, 2025 10:37
@crisbeto crisbeto requested review from wagnermaciel and removed request for a team March 13, 2025 10:37
Adds the following features to the button:
* Allows the appearance of a button to be set dynamically using the `matButton` input.
* Aligns the terminology with the M3 spec. Currently the names are derived from an old spec.
* Adds the ability to set the default appearance for buttons.
* Adds a `matIconButton` selector to the icon button for consistency.
* Adds a `matFab` selector to the FAB for consistency.
* Adds a `matMiniFab` selector to the mini FAB for consistency.

All of these changes are backwards-compatible and allow us to evolve the button in the future.

Fixes angular#15367.
Fixes angular#29841.
@crisbeto crisbeto force-pushed the 15367/dynamic-button-type branch 3 times, most recently from bacfd17 to cea2075 Compare March 13, 2025 18:42
Updates the button harness to align with the terminology for button appearances.

BREAKING CHANGE:
* `ButtonVariant` which is returned by `MatButtonHarness.getVariant` no longer includes the appearance of the button. Use `MatButtonHarness.getAppearance` instead.
Reworks the various places where we have buttons to use the new selectors.
@crisbeto crisbeto force-pushed the 15367/dynamic-button-type branch from cea2075 to 2347d69 Compare March 13, 2025 20:30
const inferredAppearance = _inferAppearance(element);

// This class is common across all the appearances so we add it ahead of time.
element.classList.add('mdc-button');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be easier to just make this a host binding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to match the behavior of what we do at the moment. Moving it into a host binding can change the order in which the class is applied which can break internal clients and I was trying to minimize the amount of breakages. I can try moving it in a follow-up. We'll have to shuffle around the host object a bit anyways if we want to get type checking support for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay that makes sense

@crisbeto crisbeto removed the request for review from wagnermaciel March 14, 2025 06:03
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Mar 14, 2025
@crisbeto crisbeto merged commit 103ac7c into angular:main Mar 14, 2025
21 checks passed
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 14, 2025
Following up on angular#30626 (comment), these changes move the button's shared host bindings into the base class and turn some classes that are added manually into host bindings.

The previous approach was only necessary with ViewEngine, because it wasn't doing inheritance of host bindings correctly and wasn't merging classes. These are no longer issues in Ivy.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 14, 2025
Following up on angular#30626 (comment), these changes move the button's shared host bindings into the base class and turn some classes that are added manually into host bindings.

The previous approach was only necessary with ViewEngine, because it wasn't doing inheritance of host bindings correctly and wasn't merging classes. These are no longer issues in Ivy.

Finally, these changes allow us to take advantage of an upcoming framework feature: angular/angular#60267
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 14, 2025
Following up on angular#30626 (comment), these changes move the button's shared host bindings into the base class and turn some classes that are added manually into host bindings.

The previous approach was only necessary with ViewEngine, because it wasn't doing inheritance of host bindings correctly and wasn't merging classes. These are no longer issues in Ivy.

Finally, these changes allow us to take advantage of an upcoming framework feature: angular/angular#60267
crisbeto added a commit that referenced this pull request Mar 17, 2025
Following up on #30626 (comment), these changes move the button's shared host bindings into the base class and turn some classes that are added manually into host bindings.

The previous approach was only necessary with ViewEngine, because it wasn't doing inheritance of host bindings correctly and wasn't merging classes. These are no longer issues in Ivy.

Finally, these changes allow us to take advantage of an upcoming framework feature: angular/angular#60267
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: material/button detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit merge: preserve commits When the PR is merged, a rebase and merge should be performed target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(buttons): align button types with m3 specs Set button type dynamicaly
2 participants