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

Mod 9702 add button component to cards #163

Merged
merged 14 commits into from
Apr 21, 2023

Conversation

chasls2
Copy link
Contributor

@chasls2 chasls2 commented Apr 14, 2023

Check List Before Merging

Author

  • Story Book has been rebuilt (new build artifacts in /docs)
  • Components have been rebuilt (new build artifacts in /dist)

👆These can be done by executing npm run build and committing the build artifacts.

  • All new components have been exported in /index.js (if applicable)
  • Design Review Complete
  • Relevant Draft release notes have been updated describing these changes (use this template to document your changes) (if applicable)
  • Integration Status Updated on relevant documentation page (if applicable)
  • index.d.ts updated with new prop types defined for new components. (if applicable)

Reviewer

  • Code review

@nick-torres nick-torres self-assigned this Apr 17, 2023
@nick-torres
Copy link
Contributor

you'll also have to update the version in package.json

className={`card__button--secondary ${variantMapper[variant]} ${customClassName}`}
aria-label={`${text}`}
disabled={disabled}
onKeyUp={(e) => handleKeyUp(e)}
onClick={action}>
{text || children}
</button>
</button> */}
<Button onKeyUp={(e) => handleKeyUp(e)} onClick={action} copy={text || children} buttonTitle={text || children} buttonSize="md" buttonType="primary" backgroundColor="light" />
Copy link
Contributor

Choose a reason for hiding this comment

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

i would think you would need to pass in the buttonType here? are they all primary buttons?

@nick-torres
Copy link
Contributor

do you think the intention of this ticket is to replace these with 'Button' too?

return ( <div className="card__button"> {govLink ? ( <div className={card__button--secondary ${variantMapper[variant]}} role="button" tabIndex="0" aria-label={${text}}> <a target="_blank" rel="noopener noreferrer" onKeyUp={(e) => handleKeyUp(e)} onClick={action} href={link}> {text} </a> </div> ) : ( <a className={card__button--secondary ${variantMapper[variant]} ${customClassName}} role="button" tabIndex={disabled ? "-1" : "0"} aria-label={${text}} href={link} onKeyUp={(e) => handleKeyUp(e)} onClick={action}> {text || children} </a> )} </div> );

@nick-torres
Copy link
Contributor

this is what the homepage looks like

image

@chasls2
Copy link
Contributor Author

chasls2 commented Apr 18, 2023

yeah, I added the buttons then make sure the styling matches, then Ill get rid of the other button

@chasls2
Copy link
Contributor Author

chasls2 commented Apr 21, 2023

Most of the styling additional styling is on usas

Copy link
Contributor

Choose a reason for hiding this comment

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

delete this file

onClick={action}>
{text || children}
</button>
<Button onKeyUp={(e) => handleKeyUp(e)} onClick={action} copy={text || children} buttonTitle={text || children} buttonSize="md" buttonType={variantMapper[variant] === undefined ? "secondary" : variantMapper[variant]} backgroundColor="light" textAlignment="center" />
Copy link
Contributor

Choose a reason for hiding this comment

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

are they always medium sized?

@nick-torres nick-torres merged commit 74d95ef into master Apr 21, 2023
@nick-torres nick-torres deleted the Mod-9702-add-button-componenet-to-cards branch April 15, 2024 14:52
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.

2 participants