Skip to content
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

fix: resolve issue with button variant detection #607

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

shon-button
Copy link

๐Ÿ“ Problem: #584
The previous code did not handle the case where 'args.variant' was undefined, leading to a 'Cannot read properties of undefined' error.

๐Ÿ› ๏ธ Solution:
Added a check to ensure 'args.variant' is defined before using it in the conditional logic. If 'args.variant' is undefined, it defaults to an empty string, preventing the error and allowing proper detection of the variant.

๐Ÿš€ Impact:
This fix ensures that the button variant detection works correctly, providing a smoother user experience and preventing potential errors.

๐Ÿ“ Problem:
The previous code did not handle the case where 'args.variant' was undefined, leading to a 'Cannot read properties of undefined' error.

๐Ÿ› ๏ธ Solution:
Added a check to ensure 'args.variant' is defined before using it in the conditional logic. If 'args.variant' is undefined, it defaults to an empty string, preventing the error and allowing proper detection of the variant.

๐Ÿš€ Impact:
This fix ensures that the button variant detection works correctly, providing a smoother user experience and preventing potential errors.
@shon-button shon-button linked an issue Aug 3, 2023 that may be closed by this pull request
@joshgamache
Copy link
Contributor

CommitLint is pretty strict on this repo, from what I remember. It only allows single-line commits, doesn't like the emojis (which I personally love having in messages, so yay to you!), and the "subject required" needs the PR title to be formatted with lowercase: fix: resolve issue with button variant detection.

If we ever swing back into working regularly in this repo, likely something worth discussing.

Copy link
Contributor

@joshgamache joshgamache left a comment

Choose a reason for hiding this comment

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

Simple and effective fix, nice work! ๐ŸŒฎ

@joshgamache joshgamache changed the title ๐Ÿ› Fix: Resolve issue with button variant detection fix: resolve issue with button variant detection Aug 3, 2023
@shon-button
Copy link
Author

CommitLint is pretty strict on this repo, from what I remember. It only allows single-line commits, doesn't like the emojis (which I personally love having in messages, so yay to you!), and the "subject required" needs the PR title to be formatted with lowercase: fix: resolve issue with button variant detection.

If we ever swing back into working regularly in this repo, likely something worth discussing.

Hi Josh,

appreciate the insight to the issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display issue with BCGov Button component in Storybook
2 participants