-
Notifications
You must be signed in to change notification settings - Fork 8
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
375-refactor: Subtitle component refactor #401
Changes from all commits
a02c4de
9673dc1
75279cf
a297a85
76f7dc1
7dd2600
ec16d59
f2b3313
36a01f4
05d0910
9c586ed
658c589
fb557a3
fb179cf
e0ac8c1
562fed2
3648c52
628d8f5
fff3924
f9c616f
ca5c092
58262dc
784c9b9
a759632
9b75b71
2c7e7fc
defad00
e2471d4
e5a0bcf
ff10470
ab009dc
0d111da
b115403
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,9 @@ export const contentMap: ContentMap = { | |
Be well-prepared to pass the "AWS Certified Developer - Associate" certification | ||
and confidently apply your skills in real-world projects by the end of the course. | ||
</Paragraph>, | ||
<Subtitle key="aws cloud dev 03" text="Course highlights:" />, | ||
<Subtitle key="aws cloud dev 03"> | ||
Course highlights: | ||
</Subtitle>, | ||
<List | ||
key="aws cloud dev 04" | ||
data={[ | ||
|
@@ -197,7 +199,9 @@ export const contentMap: ContentMap = { | |
<Paragraph key="js / front-end pre-school ru 01"> | ||
<span>ΠΠ²Π΅Π΄Π΅Π½ΠΈΠ΅ Π² RS School:</span> | ||
<List | ||
data={['ΠΠ½Π°ΠΊΠΎΠΌΡΡΠ²ΠΎ ΡΠΎ ΡΠΊΠΎΠ»ΠΎΠΉ, ΠΏΡΠΎΡΠ΅ΡΡΠΈΠ΅ΠΉ JS/Front-end ΡΠ°Π·ΡΠ°Π±ΠΎΡΡΠΈΠΊΠ° ΠΈ ΡΠΈΡΡΠ΅ΠΌΠΎΠΉ ΠΊΠΎΠ½ΡΡΠΎΠ»Ρ Π²Π΅ΡΡΠΈΠΉ Git.']} | ||
data={[ | ||
'ΠΠ½Π°ΠΊΠΎΠΌΡΡΠ²ΠΎ ΡΠΎ ΡΠΊΠΎΠ»ΠΎΠΉ, ΠΏΡΠΎΡΠ΅ΡΡΠΈΠ΅ΠΉ JS/Front-end ΡΠ°Π·ΡΠ°Π±ΠΎΡΡΠΈΠΊΠ° ΠΈ ΡΠΈΡΡΠ΅ΠΌΠΎΠΉ ΠΊΠΎΠ½ΡΡΠΎΠ»Ρ Π²Π΅ΡΡΠΈΠΉ Git.', | ||
]} | ||
marked={false} | ||
/> | ||
</Paragraph>, | ||
|
@@ -230,7 +234,10 @@ export const contentMap: ContentMap = { | |
</Paragraph>, | ||
<Paragraph key="js / front-end pre-school ru 05"> | ||
<span>ΠΡΠΎΠ³ΠΎΠ²Π°Ρ Π°ΡΡΠ΅ΡΡΠ°ΡΠΈΡ:</span> | ||
<List data={['ΠΡΠΎΡΡ-ΡΠ΅ΠΊ ΠΏΡΠΎΠ΅ΠΊΡΠΎΠ², ΡΠ΅ΡΡΡ ΠΈ ΡΠ΅Π²ΡΡ ΠΊΠΎΠ΄Π°. ΠΡΠ΄Π°ΡΠ° ΡΠ΅ΡΡΠΈΡΠΈΠΊΠ°ΡΠ°.']} marked={false} /> | ||
<List | ||
data={['ΠΡΠΎΡΡ-ΡΠ΅ΠΊ ΠΏΡΠΎΠ΅ΠΊΡΠΎΠ², ΡΠ΅ΡΡΡ ΠΈ ΡΠ΅Π²ΡΡ ΠΊΠΎΠ΄Π°. ΠΡΠ΄Π°ΡΠ° ΡΠ΅ΡΡΠΈΡΠΈΠΊΠ°ΡΠ°.']} | ||
marked={false} | ||
/> | ||
</Paragraph>, | ||
], | ||
image: jsImg, | ||
|
@@ -254,10 +261,10 @@ export const contentMap: ContentMap = { | |
'Understanding of the REST', | ||
]} | ||
/>, | ||
<Subtitle | ||
key="react 03" | ||
text="Attention! Mentors on this course will be first assigned to the graduates of the RS School Stage #2." | ||
/>, | ||
<Subtitle key="react 03"> | ||
Attention! Mentors on this course will be first assigned to the graduates of the RS School | ||
Stage #2. | ||
</Subtitle>, | ||
], | ||
image: reactEnImg, | ||
}, | ||
|
@@ -289,13 +296,12 @@ export const contentMap: ContentMap = { | |
title: 'Details', | ||
content: [ | ||
<Paragraph key="aws devops 01"> | ||
If you are looking for an entry point to kickstart your IT career as a DevOps engineer, | ||
then this AWS course challenge is what you need. | ||
If you are looking for an entry point to kickstart your IT career as a DevOps engineer, then | ||
this AWS course challenge is what you need. | ||
</Paragraph>, | ||
<Subtitle | ||
key="aws devops 02" | ||
text="Showcase your level of expertise and join this expert-led program to:" | ||
/>, | ||
<Subtitle key="aws devops 02"> | ||
Showcase your level of expertise and join this expert-led program to: | ||
</Subtitle>, | ||
<List | ||
key="aws devops 03" | ||
data={[ | ||
|
@@ -308,10 +314,9 @@ export const contentMap: ContentMap = { | |
'Become a DevOps engineer who is ready to face engineering challenges', | ||
]} | ||
/>, | ||
<Subtitle | ||
key="aws devops 04" | ||
text="What do we offer?" | ||
/>, | ||
<Subtitle key="aws devops 04"> | ||
What do we offer? | ||
</Subtitle>, | ||
Comment on lines
+317
to
+319
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too |
||
<List | ||
key="aws devops 05" | ||
data={[ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
.subtitle { | ||
ansivgit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
margin: 0; | ||
natanchik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
font-weight: $font-weight-medium; | ||
|
||
&.extra-small-font-size { | ||
font-size: 20px; | ||
line-height: 28px; | ||
|
||
@include media-tablet { | ||
font-size: 18px; | ||
line-height: 24px; | ||
} | ||
} | ||
|
||
&.small-font-size { | ||
font-size: 24px; | ||
line-height: 32px; | ||
|
||
@include media-tablet { | ||
font-size: 20px; | ||
line-height: 28px; | ||
} | ||
} | ||
|
||
&.medium-font-size { | ||
font-size: 28px; | ||
line-height: 36px; | ||
|
||
@include media-tablet { | ||
font-size: 24px; | ||
line-height: 32px; | ||
} | ||
} | ||
|
||
&.large-font-size { | ||
font-size: 32px; | ||
line-height: 40px; | ||
|
||
@include media-tablet { | ||
font-size: 28px; | ||
line-height: 36px; | ||
} | ||
} | ||
|
||
&.extra-large-font-size { | ||
font-size: 36px; | ||
line-height: 44px; | ||
|
||
@include media-tablet { | ||
font-size: 32px; | ||
line-height: 36px; | ||
} | ||
} | ||
seygorin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
&.gray-600 { | ||
color: $color-gray-600; | ||
} | ||
|
||
&.black { | ||
color: $color-black; | ||
} | ||
} | ||
KristiBo marked this conversation as resolved.
Show resolved
Hide resolved
|
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,19 +1,86 @@ | ||||||
import { render, screen } from '@testing-library/react'; | ||||||
import { cleanup, render, screen } from '@testing-library/react'; | ||||||
import { describe, expect, it } from 'vitest'; | ||||||
import { Subtitle } from './subtitle'; | ||||||
import { Subtitle, cx } from './subtitle'; | ||||||
|
||||||
describe('Subtitle component', () => { | ||||||
it('renders without crashing', () => { | ||||||
render(<Subtitle text="Test Subtitle" />); | ||||||
const subtitle = screen.getByText('Test Subtitle'); | ||||||
render(<Subtitle>Test Subtitle</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toBeInTheDocument(); | ||||||
}); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
it('displays correct text', () => { | ||||||
render(<Subtitle text="Another Test Subtitle" />); | ||||||
const text = screen.getByText('Another Test Subtitle'); | ||||||
const testText = 'Another Test Subtitle'; | ||||||
|
||||||
SpaNb4 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
expect(text).toBeInTheDocument(); | ||||||
render(<Subtitle>{testText}</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveTextContent(testText); | ||||||
}); | ||||||
|
||||||
it('applies medium font size class by default', () => { | ||||||
render(<Subtitle>Default Font Size</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass(cx('medium-font-size')); | ||||||
}); | ||||||
|
||||||
it('applies correct font size classes when specified', () => { | ||||||
const sizes = ['extra-small', 'small', 'medium', 'large', 'extra-large'] as const; | ||||||
|
||||||
sizes.forEach((size) => { | ||||||
const { getByTestId } = render( | ||||||
<Subtitle fontSize={size}> | ||||||
{size} | ||||||
Font Size | ||||||
</Subtitle>, | ||||||
); | ||||||
const subtitle = getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass(cx(`${size}-font-size`)); | ||||||
cleanup(); | ||||||
}); | ||||||
}); | ||||||
|
||||||
it('applies gray color class by default', () => { | ||||||
render(<Subtitle>Default Color</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass(cx('gray-600')); | ||||||
}); | ||||||
|
||||||
it('applies black color class when specified', () => { | ||||||
render(<Subtitle color="black">Black Color</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass(cx('black')); | ||||||
}); | ||||||
|
||||||
it('applies custom className when provided', () => { | ||||||
render(<Subtitle className="custom-class">Custom Class</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass('custom-class'); | ||||||
}); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
it('applies multiple variant classes correctly', () => { | ||||||
render( | ||||||
<Subtitle fontSize="large" color="black" className="custom-class"> | ||||||
Multiple Variants | ||||||
</Subtitle>, | ||||||
); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle).toHaveClass(cx('large-font-size')); | ||||||
expect(subtitle).toHaveClass(cx('black')); | ||||||
expect(subtitle).toHaveClass('custom-class'); | ||||||
}); | ||||||
|
||||||
it('renders as h3 element', () => { | ||||||
render(<Subtitle>H3 Element</Subtitle>); | ||||||
const subtitle = screen.getByTestId('subtitle'); | ||||||
|
||||||
expect(subtitle.tagName).toBe('H3'); | ||||||
}); | ||||||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,45 @@ | ||
import './subtitle.scss'; | ||
import { HTMLAttributes } from 'react'; | ||
import { type VariantProps, cva } from 'class-variance-authority'; | ||
import classNames from 'classnames/bind'; | ||
|
||
interface SubtitleProps { | ||
text: string; | ||
type?: string; | ||
} | ||
import styles from './subtitle.module.scss'; | ||
|
||
export const Subtitle = ({ text, type = '' }: SubtitleProps) => ( | ||
<div className={`subtitle ${type}`}>{text}</div> | ||
); | ||
type SubtitleProps = Pick<HTMLAttributes<HTMLHeadingElement>, 'className' | 'children'> & | ||
VariantProps<typeof subtitleVariants>; | ||
|
||
ansivgit marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have seen the the same problem in our codebase before, but are we sure to use type |
||
export const cx = classNames.bind(styles); | ||
|
||
const subtitleVariants = cva(cx('subtitle'), { | ||
variants: { | ||
fontSize: { | ||
'extra-small': cx('extra-small-font-size'), | ||
small: cx('small-font-size'), | ||
medium: cx('medium-font-size'), | ||
large: cx('large-font-size'), | ||
'extra-large': cx('extra-large-font-size'), | ||
}, | ||
color: { | ||
gray: cx('gray-600'), | ||
black: cx('black'), | ||
}, | ||
}, | ||
defaultVariants: { | ||
fontSize: 'medium', | ||
color: 'gray', | ||
}, | ||
}); | ||
|
||
export const Subtitle = ({ children, fontSize, color, className }: SubtitleProps) => { | ||
return ( | ||
<h3 | ||
className={subtitleVariants({ | ||
fontSize, | ||
color, | ||
className, | ||
})} | ||
data-testid="subtitle" | ||
> | ||
{children} | ||
</h3> | ||
); | ||
}; |
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.
Here too