-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Timeline][lab] Fix position alternate-reverse
generated classname
#37678
[Timeline][lab] Fix position alternate-reverse
generated classname
#37678
Conversation
Netlify deploy previewhttps://deploy-preview-37678--material-ui.netlify.app/ Bundle size report |
export default function convertTimelinePositionToClass(position: string): string { | ||
return position === 'alternate-reverse' | ||
? 'positionAlternateReverse' | ||
: `position${capitalize(position)}`; |
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'm unsure whether I should create a pascalize function specifically to convert kebab-case to PascalCase for this situation or simply keep the string positionAlternateReverse
hardcoded here.
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.
We can add a function to convert kebab case to pascal case in the utils package and use it in all occurrences where needed. Nice catch, btw 👍
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.
Since there is only one occurrence in the whole code-base, would it be worth it to create a function?
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 checked and found that it is only needed here. I think it is not worth it to create a util function just for this and so I am keeping the string hard-coded.
Hi @ZeeshanTamboli , is this the cause of |
alternate-reverse
generated classnamealternate-reverse
generated classname
@Sutar210599 It is not related to this fix. The issue is that we added this feature in #37311 in the last release but we forgot to bump the version of |
Ok, thanks. |
The classname generated for
alternate-reverse
class after adding it in #37311 should be.MuiTimeline-positionAlternateReverse
, not.MuiTimeline-positionAlternate-reverse
. The capitalize function only capitalizes the first letter of the first word and does not convert kebab-case to PascalCase.