feat(ChangelogDialog): change style, add link, events #105
Conversation
|
Preview is ready. |
Feverqwe
force-pushed
the
rndnm/CLOUDFRONT-16740/updChangelogDialog
branch
from
2784061 to
26417c2
Compare
No, we don't use it yet. And just add one optional prop |
You don't use, but others could |
Feverqwe
force-pushed
the
rndnm/CLOUDFRONT-16740/updChangelogDialog
branch
from
26417c2 to
f3666a4
Compare
|
@Feverqwe Removing prop is breaking change. Mark as deprecated and remove in the next major release |
Ok, I remove breaking changes, and just add link feature and change style |
| } | ||
|
|
||
| &__title { | ||
| margin: 0; | ||
| @include mixins.text-subheader-3(); | ||
| @include mixins.text-subheader-2(); | ||
| font-weight: 500; |
There was a problem hiding this comment.
mixin already has font-weight. Besides it uses variable for that, not a fixed value
amje
changed the title
Feverqwe
force-pushed
the
rndnm/CLOUDFRONT-16740/updChangelogDialog
branch
from
7286dee to
a7d7084
Compare
Feverqwe
force-pushed
the
rndnm/CLOUDFRONT-16740/updChangelogDialog
branch
from
a7d7084 to
fb44861
Compare
|
@amje I'm add events, rm metrics |
Feverqwe
changed the title
| }: ChangelogDialogProps) { | ||
| const idRef = React.useRef<number>(); | ||
| idRef.current = idRef.current || getNextId(); | ||
| const dialogCaptionId = `changelog-dialog-title-${idRef.current}`; | ||
|
|
||
| useEffect(() => { | ||
| if (!open) return; |
There was a problem hiding this comment.
What the reason to add callback, if you pass open from outside anyway?
There was a problem hiding this comment.
Looks like there is no need to keep this code inside the component
There was a problem hiding this comment.
Yes, you are right.
| @@ -21,7 +21,9 @@ export interface ChangelogDialogProps { | |||
| items: ChangelogItem[]; | |||
| disableBodyScrollLock?: boolean; | |||
| disableOutsideClick?: boolean; | |||
| onOpen: () => void; | |||
There was a problem hiding this comment.
Remove, will do in parent component
| }: ChangelogDialogProps) { | ||
| const idRef = React.useRef<number>(); | ||
| idRef.current = idRef.current || getNextId(); | ||
| const dialogCaptionId = `changelog-dialog-title-${idRef.current}`; | ||
|
|
||
| useEffect(() => { | ||
| if (!open) return; |
There was a problem hiding this comment.
Looks like there is no need to keep this code inside the component
| @@ -17,17 +17,17 @@ $metaWidth: 124px; | |||
| } | |||
|
|
|||
| &__label-new { | |||
| margin-top: 4px; | |||
| margin-top: 8px; | |||
There was a problem hiding this comment.
I guess we should use --g-spacing-2
| } | ||
|
|
||
| &__content { | ||
| flex: 1; | ||
| margin-left: 16px; | ||
| margin-left: 20px; |
There was a problem hiding this comment.
I guess we should use --g-spacing-5
There was a problem hiding this comment.
Replace this and other places too.
No description provided.