chore: replaced Badge references with MDS Badge #2933
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📦 Next.js Bundle AnalysisThis analysis was generated by the next.js bundle analysis action 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! One Hundred Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. |
| export function isInjectedPermalink(child: ReactNode): boolean { | ||
| if (!isValidElement(child)) return false | ||
|
|
||
| // props is unknown-ish, so narrow safely | ||
| const className = (child.props as { className?: unknown }).className | ||
| return className === "__permalink-h" |
There was a problem hiding this comment.
The former typing of child as ReactChild | ReactFragment | ReactPortal | PromiseLikeOfReactNode was causing issues with the bump to @types/react v18.2.68.
This change provides a cleaner, React 18 friendly version with these benefits:
- Works across React 18 and many
@types/reactpatch versions - Doesn’t rely on deprecated-ish unions like
ReactChild - Avoids the whole PromiseLike thing entirely which is only an experimental or React 19 feature.
- Uses
React.isValidElementinstead ofpropsin child hacks
| <Text.DisplayExpressive | ||
| tag={`h${level}`} | ||
| tag={tag} | ||
| size="400" | ||
| weight={weight} | ||
| ref={ref} |
There was a problem hiding this comment.
Better, and more strict, polymorphic typing was introduced for MDS Text components in the latest release. This requires the heading tag to be cast as only valid HTML tags.
| &::after { | ||
| content: ""; | ||
| position: absolute; | ||
| right: -24px; | ||
| top: 0; | ||
| bottom: 0; | ||
| width: 24px; | ||
| } |
There was a problem hiding this comment.
Just a little extra fix thrown in here to make it easier to move the mouse cursor from the top-left HashiCorp logo to the "popover" nav panel.
| ref={(element: HTMLButtonElement | null) => { | ||
| buttonElements.current[index] = element | ||
| }} |
There was a problem hiding this comment.
Callback refs in React must return void, but the ref callback here was returning the assigned element because the assignment expression evaluates to element.
Some older @types/react versions were looser about callback ref return types or type inference didn’t catch the expression-return. Our newer version is stricter (and closer to React’s intended contract).
| > | ||
| <span className={s.label}> | ||
| {icon} | ||
| {icon ? <FlightIcon name={icon} size={16} /> : null} |
There was a problem hiding this comment.
Since icon here is now the icon name from the FlightIconName string literal union type, we now need to render the icon svg here with the MDS FlightIcon component by passing icon to the name prop.
| <FlightIcon | ||
| title="Interactive" | ||
| name="terminal-screen" | ||
| size={12} | ||
| color="var(--mds-color-foreground-primary" | ||
| className={s.interactiveIcon} |
There was a problem hiding this comment.
Rather than adding an override/modification to the Badge component to just display an icon we just render the FlightIcon component.
| {text, ...rest}: Omit<BadgeProps, 'type' | 'className' | 'size'> | ||
| ) { | ||
| return <Badge {...props} type="base" className={s.badge} size="small" /> | ||
| return <Badge text={text} className={s.badge} size="small" {...rest} /> |
There was a problem hiding this comment.
Instead of relying on a type="base" variant of badge, since it was only used in this one place, we are just applying a very small style override. This could be done differently in the future but this is fine for now.
| const Adapter: QueryParamAdapterComponent = (props) => ( | ||
| <NextAdapter {...props} /> | ||
| ) | ||
|
|
There was a problem hiding this comment.
use-query-params types the adapter prop as a component that returns a ReactElement. The NextAdapter value is typed as a memoized component (MemoExoticComponent<...>), and in React 18’s types that shape is being treated as returning a ReactNode. Since ReactNode can be string, typescript was yelling that "this might be a string, and I require a ReactElement.”
By using a wrapper component pattern typescript is forced to see a normal function component, and it satisfies QueryParamProvider’s adapter type.
For more info see https://www.npmjs.com/package/next-query-params/v/4.1.0
| .badgeContainer { | ||
| margin-left: 0.3333em; | ||
| position: relative; | ||
| top: -0.1em; |
There was a problem hiding this comment.
Not needed with how the MDS badge alignment works
LeahMarieBush
left a comment
LeahMarieBush
left a comment
There was a problem hiding this comment.
Looks good! One small comment about centering of the badges in the search results but other than that it looks great! I appreciate the product dropdown opening fix too 🙌
|
|
||
| import { ReactElement } from 'react' | ||
| import Badge from 'components/badge' | ||
| import { Badge, type FlightIconName } from '@hashicorp/mds-react/components' |
There was a problem hiding this comment.
The badges in the preview look a bit off center compared to prod:
There was a problem hiding this comment.
I updated the css in the mds-react package to not include vertical-align: middle on the badge element. This wasn't necessary for .com and was causing this alignment issue in devdot.
mikegolus
force-pushed
the
mikegolus/implement-mds-badge
branch
from
4c9060f to
c8739e2
Compare
|
This PR is stale because it has been open 20 days with no activity. It will be closed in 5 days unless you remove the |
mikegolus
force-pushed
the
mikegolus/implement-mds-badge
branch
from
0c2ba57 to
487e746
Compare
2 participants
🔗 Relevant links
🗒️ What
This PR is to replace the local instance (
src/components/badge) of theBadgecomponent in devdot with the MDSBadge.@types/reactback to the dependency list but at v18.2.68 instead of v18.2.6 to better align with the web monorepo version@hashicorp/platform-packer-pluginsand thesrc/views/packer-pluginsdirectory as it was not in useBadgecomponent'siconprop takes an icon name from the FlightIconName string literal union type rather than requiring an icon component be passed to it. All instances of imported icon components related to badge icons have been refactored to use FlightIconNameBadgehave been replaced with the MDSBadgesrc/components/badge/types.tshave been replaced with MDSBadgePropsBadgefromsrc/components/badgehas been deletedBadgeattempted to.aria-labelfor accessible text, but this is not valid for non-interactive elements (see https://benmyers.dev/blog/dont-use-aria-label-on-static-text-elements/)spanwhen accessible text is providedneutral-dark-modewas equivalent to colorneutraland typeinverted.neutral-dark-modehas been removed from the MDS badge in favor of using neutral invertedbackground-colorbeing overwritten withvar(--mds-color-surface-primary)have been removed since this is now the default background color for these specific badge instancesNumericBadgecomponent fromsrc/components/numeric-badgehas been deleted as it was not in use.📸 Screenshots
Screenshots with the search modal open were chosen because they have the most visible instances of the badge component for review.
Dark Mode
Before:
After:
Light Mode
Before:
After:
🧪 Testing