feat(onboarding): improve onboarding template component #13134
Conversation
| </div> | ||
| )} | ||
| <section className="flex flex-col items-center mt-8"> | ||
| <img className="max-h-28" src={placeholderSrc} {...img} /> |
There was a problem hiding this comment.
nitpick(blocking):placeHolderSrc is just an example image. We have to consider the src from img tag if it's present. This causes confusion if it display the img.src or placeholderSrc.
There was a problem hiding this comment.
The img.src overwrite the src already set. Same if className exists on img props.
You want still I add more logic for readability ?
There was a problem hiding this comment.
Maybe we should concat img.className with the current className instead of overwriting it completely though
There was a problem hiding this comment.
It's a choice.
- No do that allow to have more flexibility.
- Do that add behavior by default but can add adverse reaction on specific case.
Example : On MANAGER-14496 see #13146 . We have image with text title. So I want set max-h-36
It's easier it className is overwrite. And if we add a className. I think you can be responsible of all display rules.
After. May be export DEFAULT_MAX_HEIGHT_CLASS if we want add possibility to extends on parent component ?
I try to keep simple. That's how I did it. But not hesitate if you think it's better to do otherwise
There was a problem hiding this comment.
I agree with @chipp972 for concatenating the classname.
There was a problem hiding this comment.
Done. Not hesitate to say me if change is good for you ;)
tibs245
dismissed stale reviews from chipp972 and fredericvilcot
via
38c94e2
tibs245
force-pushed
the
feat/onboarding-template
branch
2 times, most recently
from
38c94e2
to
54e47b7
Compare
packages/manager-react-components/src/components/templates/onboarding/onboarding.component.tsx
Outdated
Show resolved
Hide resolved
tibs245
force-pushed
the
feat/onboarding-template
branch
from
54e47b7
to
c6199bc
Compare
github-actions
bot
added
the
has conflicts
tibs245
force-pushed
the
feat/onboarding-template
branch
from
c6199bc
to
8e37c2a
Compare
github-actions
bot
removed
the
has conflicts
packages/manager-react-components/src/components/templates/onboarding/onboarding.component.tsx
Outdated
Show resolved
Hide resolved
Use React.ComponentProps to improve flexibility of image rules Signed-off-by: Thibault Barske <[email protected]>
tibs245
force-pushed
the
feat/onboarding-template
branch
from
8e37c2a
to
4caca80
Compare
|
Use React.ComponentProps to improve flexibility of image rules Signed-off-by: Thibault Barske <[email protected]>
developOnly FR translations have been updatedTicket reference is mentioned in linked commits (internal only)Breaking change is mentioned in relevant commitsDescription
Improve img props type to add more flexibility on image rules and be more readable