-
Notifications
You must be signed in to change notification settings - Fork 20
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(cc-flex-gap): Remove the component and replace it with CSS #768
Conversation
🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/cc-flex-gap/remove/index.html. This preview will be deleted once this PR is closed. |
fd773d3
to
49a8ad6
Compare
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.
Well done Flo! I've got nothing to say.
49a8ad6
to
1eb3f77
Compare
1b2277c
to
39080de
Compare
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.
Hello, nice job on changing them, I've checked the components as you asked and I found some things that I left below. Otherwise good job! 💪
- question:
cc-header-orga
has an alignment difference, is this voluntary? - nitpick(non-blocking): there is a slight alignment difference for
cc-env-var-input
but it's barely noticeable so we can leave it like that I guess. As thecc-env-var-form
contains somecc-env-var-input
you can see it in the component too. - issue: the alignment with the zone is broken on
cc-addon-linked-apps
(:warning: Watch out you put cc-addon-credentials
twice in the components to review visually 😉)
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.
A very thorough PR, as usual: very nice job!
I have the same feedback as Mathieu on:
cc-addon-linked-apps
:cc-zone
alignment is off with the beginning of the linecc-header-orga
: space is missing between the main name and the badge
I'll also add:
cc-header-addon
: breakpoints when content should wrap is broken
Thanks for your reviews @Galimede @roberttran-cc!
I made separate commits for your fixes so you can check them easily ( |
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.
cc-header-orga
, cc-header-addon
, cc-env-var-input
: perfect!
cc-addon-linked-apps
: I made a suggestion on how to fix the issue, tell me what you think about it!
91606a3
to
259173b
Compare
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.
LGTM, GG! 💪
BREAKING CHANGE
f9c6835
to
51666f6
Compare
51666f6
to
7459637
Compare
🔎 The preview has been automatically deleted. |
Fixes #769
What does this PR do?
cc-flex-gap
component,Note:
cc-pricing-*
components andcc-zone
. It should be merged after the pricing PR.Things to check
TODO (Flo)