DRAFT 589-feat: Aws badge widgets #842
Conversation
|
Lighthouse Report:
|
ed632ca to
ed1f3aa
Compare
|
run visual now |
|
Lighthouse Report:
|
|
Lighthouse Report:
|
| @@ -30,18 +23,14 @@ export { | |||
| RS_DOCS_EN_LINK, | |||
| RS_DOCS_TELEGRAM_CHATS_LINK, | |||
| } from './communication.data'; | |||
|
|
|||
| export { type Benefit } from './benefit-mentorship.data'; | |||
There was a problem hiding this comment.
| export { type Benefit } from './benefit-mentorship.data'; | |
| export type { Benefit } from './benefit-mentorship.data'; |
There was a problem hiding this comment.
@ansivgit We should probably add a corresponding eslint rule
There was a problem hiding this comment.
Now this component can't be used anywhere else. I suggest replacing it with the Badge component, remove aws name-parts from it and moving the all data (title, text, badge params) to the folder dev-data
|
Lighthouse Report:
|
dev-data/badge.data.tsx
Outdated
There was a problem hiding this comment.
It's not .data, it's component. Put it in shared folder
dev-data/badge.data.tsx
Outdated
| import { Paragraph } from '@/shared/ui/paragraph'; | ||
| import { WidgetTitle } from '@/shared/ui/widget-title'; | ||
|
|
||
| type BadgeData = { |
There was a problem hiding this comment.
| type BadgeData = { | |
| type BadgeProps = { |
dev-data/badge.data.tsx
Outdated
| return ( | ||
| <section | ||
| className={cx(data.className, 'container')} | ||
| data-testid={data.className} |
There was a problem hiding this comment.
| data-testid={data.className} | |
| data-testid="badge" |
dev-data/badge.data.tsx
Outdated
| image: StaticImageData; | ||
| alt: string; | ||
| className: string; | ||
| styles: { [key: string]: string }; |
There was a problem hiding this comment.
Why do we need styles prop here? We could use .scss as we do in other shared components
There was a problem hiding this comment.
Ok, I've read the comments, and I will make the changes.
There was a problem hiding this comment.
You could use this component as an example of file & folder structure src/shared/ui/logo
There was a problem hiding this comment.
I guess i did it.
|
Lighthouse Report:
|
src/shared/ui/badge/badge.tsx
Outdated
| }; | ||
|
|
||
| export const Badge = (data: BadgeProps) => { | ||
| const cx = classNames.bind(styles); |
There was a problem hiding this comment.
Please move cx outside the component, similar to how it's handled in other components
src/shared/ui/badge/badge.tsx
Outdated
|
|
||
| return ( | ||
| <section | ||
| className={cx('badge', 'container')} |
There was a problem hiding this comment.
badge className seems redundant to me
There was a problem hiding this comment.
Since AwsBadge is just a regular Badge component, I don't see a reason for adding separate tests for it
There was a problem hiding this comment.
I believe I've made all the necessary changes.
|
Lighthouse Report:
|
|
Changes have been moved to contentful, PR can be closed |
What type of PR is this? (select all that apply)
Description
removed specify property from TrainingProgramProps type
created a separate aws-widget folder and files for represent the aws badge data.
deleted aws badge from the list of the training programs
added it from the widgets
Related Issue Non course data in the courses content map #589
Closes Non course data in the courses content map #589
Screenshots, Recordings
Before
After
Added/updated tests?
[optional] Are there any post deployment tasks we need to perform 8000 ?
[optional] What gif best describes this PR or how it makes you feel?