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.

Sign up for GitHub

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

Jump to bottom

SearchNav feat: add new CTA and text #742

Merged
merged 5 commits into from
Oct 31, 2023
Merged

SearchNav feat: add new CTA and text #742

merged 5 commits into from
Oct 31, 2023

Conversation

OskiTheCoder
Copy link
Member

@OskiTheCoder OskiTheCoder commented Oct 27, 2023 •
edited
Loading

📝 Changes

  • adds PrimaryCTAItem
  • original CTAItem renamed to SecondaryCTAItem
  • adds EmphasizedText to handle rendering of text in LogoGroup

✅ Checklist

  • Visuals are complete and match Figma
  • Code is complete and in accordance with our style guide
  • Design and theme tokens are audited for any relevant changes
  • Unit tests are written and passing
  • TSDoc is written or updated for any component API surface area
  • Stories in Storybook accompany any relevant component changes
  • Ensure no accessibility violations are reported in Storybook
  • Specs and documentation are up-to-date
  • Cross-browser check is performed (Chrome, Safari, Firefox)
  • Changeset is added

@OskiTheCoder OskiTheCoder changed the title Searchnav new requirements SearchNav feat: add new CTA and text Oct 28, 2023
stephenjwatkins
stephenjwatkins reviewed Oct 30, 2023
View reviewed changes
easy-ui-react/src/SearchNav/SearchNav.tsx Show resolved Hide resolved
easy-ui-react/src/SearchNav/EmphasizedText.tsx Outdated Show resolved Hide resolved
easy-ui-react/src/SearchNav/LogoGroup.tsx Outdated Show resolved Hide resolved
easy-ui-react/src/SearchNav/EmphasizedText.tsx Outdated Show resolved Hide resolved
stephenjwatkins
stephenjwatkins reviewed Oct 31, 2023
View reviewed changes
easy-ui-react/src/SearchNav/SearchNav.mdx Outdated Show resolved Hide resolved
easy-ui-react/src/SearchNav/Title.tsx Outdated
Comment on lines 18 to 20
<span className={classNames(styles.title)}>
<Text variant="subtitle1">{children}</Text>
</span>
Copy link
Member

Choose a reason for hiding this comment

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

question: how would you feel about adding transform support to Text so that the intermediary component isn't needed?

something like:

image image image

Copy link
Member Author

Choose a reason for hiding this comment

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

ah great suggestion, went ahead and added it in

easy-ui-react/src/SearchNav/utilities.ts Outdated Show resolved Hide resolved
stephenjwatkins
stephenjwatkins approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@stephenjwatkins stephenjwatkins left a comment

Choose a reason for hiding this comment

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

looks good to me!

@OskiTheCoder OskiTheCoder merged commit 0cfafc2 into main Oct 31, 2023
3 checks passed
@OskiTheCoder OskiTheCoder deleted the searchnav_new_requirements branch October 31, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephenjwatkins stephenjwatkins stephenjwatkins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@OskiTheCoder @stephenjwatkins