Skip to content

Comments

fix: migrate buttondialpad#786

Merged
cscheffauer merged 3 commits intomomentum-design:masterfrom
cscheffauer:fix-migrate-buttondialpad
Apr 11, 2025
Merged

fix: migrate buttondialpad#786
cscheffauer merged 3 commits intomomentum-design:masterfrom
cscheffauer:fix-migrate-buttondialpad

Conversation

@cscheffauer
Copy link
Collaborator

Description

  • Migrating the ButtonDialpad to use the new momentum-design button
  • Removing size and children attribute to simplify component (not necessary)
  • Updating unit tests

@cscheffauer cscheffauer added the validated If the pull request is validated automation. label Apr 10, 2025
Copy link
Collaborator

@jor-row jor-row left a comment

Choose a reason for hiding this comment

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

some very small comments like unused constants that can be removed and a type issue

primaryText: `${CLASS_PREFIX}-primary-text`,
secondaryText: `${CLASS_PREFIX}-secondary-text`,
wrapper: `${CLASS_PREFIX}-wrapper`,
innerWrapper: `${CLASS_PREFIX}-inner-wrapper`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default SIZE and SIZE constants can also be removed as they are no longer needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

the default for DISABLED is only used in a test so can be removed as well

// size in scss file to 64px
size={40}
variant="tertiary"
{...(otherProps as any)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we type this otherProps? as Omit<Props, 'className'>


return (
<ButtonSimple
<MdcButton
Copy link
Collaborator

Choose a reason for hiding this comment

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

the disabled button doesn't really look disabled but i guess it was that way before so its fine as is

@cscheffauer cscheffauer merged commit c6475d5 into momentum-design:master Apr 11, 2025
5 checks passed
@github-actions
Copy link

🎉 This PR is included in version 26.201.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released validated If the pull request is validated automation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants