-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(ai-label): add ts types #17295
feat(ai-label): add ts types #17295
Conversation
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 💯
const propMappingFunction = (deprecatedValue) => { | ||
const mapping = { | ||
'top-left': 'top-start', | ||
'top-right': 'top-end', | ||
'bottom-left': 'bottom-start', | ||
'bottom-right': 'bottom-end', | ||
'left-bottom': 'left-end', | ||
'left-top': 'left-start', | ||
'right-bottom': 'right-end', | ||
'right-top': 'right-start', | ||
}; | ||
return mapping[deprecatedValue]; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker to merge at all, but at some point we should probably make this a reusable helper - we're going to be using this all over the codebase 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally forgot we have one already:
carbon/packages/react/src/tools/createPropAdapter.js
Lines 44 to 67 in afce28d
export function mapPopoverAlignProp(align) { | |
switch (align) { | |
case 'top-left': | |
return 'top-start'; | |
case 'top-right': | |
return 'top-end'; | |
case 'bottom-left': | |
return 'bottom-start'; | |
case 'bottom-right': | |
return 'bottom-end'; | |
case 'left-bottom': | |
return 'left-end'; | |
case 'left-top': | |
return 'left-start'; | |
case 'right-bottom': | |
return 'right-end'; | |
case 'right-top': | |
return 'right-start'; | |
default: | |
return align; | |
} | |
} | |
export { mapDownshiftProps }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh thanks! I updated AI label!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17295 +/- ##
==========================================
- Coverage 76.83% 76.83% -0.01%
==========================================
Files 408 408
Lines 13945 13949 +4
Branches 4323 4276 -47
==========================================
+ Hits 10715 10718 +3
- Misses 3056 3058 +2
+ Partials 174 173 -1 ☔ View full report in Codecov by Sentry. |
cfba956
Hey there! v11.66.0 was just released that references this issue/PR. |
Closes #17232
Add typescript types to AI-label
Changelog
Changed
Popover
syntax with deprecated valuesTesting / Reviewing
functionality stay the same
checks still passing
storybook looks good