-
Notifications
You must be signed in to change notification settings - Fork 14
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!: yfm instead of html #993
Conversation
Playwright Test Component is ready. |
Preview is ready. |
2e5595d
to
6f3272d
Compare
1cfa7ff
to
61d94cb
Compare
61d94cb
to
1790520
Compare
src/components/HTML/HTML.tsx
Outdated
export interface HTMLProps { | ||
children?: string; | ||
export interface HTMLExtraProps { | ||
variant?: 'span' | 'div' | 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' | 'section' | 'p'; |
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.
would it be easier to make an enum for variand use it in other places like this https://github.com/gravity-ui/page-constructor/pull/993/files#diff-8a573455e8636f1d6dd641125e9b47de96bbf75f3fdd9bff39a2233e7515144bR27 ?
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.
enum is good if value is unknown, or difficult to write. When value is simple and well-known it is better to use simple type as I do.
src/components/HTML/HTML.tsx
Outdated
export interface HTMLProps { | ||
children?: string; | ||
export interface HTMLExtraProps { | ||
variant?: 'span' | 'div' | 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' | 'section' | 'p'; |
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.
and I think we need more informative name for this props , maybe tagVariant
or tagType
because I have a question - a variant of what?))
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.
changed it to tagName
* feat: brand footer * fix: style imports * fix: requested changes * fix: split brand icon
* fix(Media): fix video microdata
f155278
to
2390740
Compare
@include heading4(); | ||
@include focusable(); | ||
@include add-specificity(&) { | ||
// @include reset-button-style(); |
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.
delete if we dont need it
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.
forgot to push changes (( Sorry
No description provided.