-
Notifications
You must be signed in to change notification settings - Fork 213
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: /kubernetes/compare redesign #14660
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## k8s-bubble-refresh #14660 +/- ##
======================================================
- Coverage 74.46% 72.58% -1.89%
======================================================
Files 107 120 +13
Lines 2847 3392 +545
Branches 948 1169 +221
======================================================
+ Hits 2120 2462 +342
- Misses 703 906 +203
Partials 24 24 |
thank youu @mtruj013, pretty much perfect! just one tiny comment: Table section:
|
Thanks @mtruj013! Just some comments on the copy:
Edit: Sorry I just checked how it looks on smaller screens and it seems to cut off the text that doesn't fit within the containers. Is it possible to wrap the text so we don't lose any content? ![]() |
Thanks for the quick changes @mtruj013, looks great! Just one comment - could you change & to "and" in the "Multi-cloud deployments & operations" heading as well? Super minor though so will add UX+1. Thanks! |
<div class="row--50-50"> | ||
<div class="col"> | ||
<h1> | ||
Kubernetes | ||
<br /> | ||
distributions comparison |
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.
Hey, did you try out hero_macro here?
https://vanillaframework.io/docs/patterns/hero
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.
Yes, but it doesn't have the option for 50/50 split with just text unfortunately
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.
ticket opened here
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.
Thanks for the ticket! I've raised a PR to implement a fix that supports 50/50 with no image.
templates/kubernetes/compare.html
Outdated
data-form-location="/shared/forms/interactive/kubernetes" | ||
data-form-id="3230" | ||
data-lp-id="6274" | ||
data-return-url="https://ubuntu.com/kubernetes/thank-you?product=kubernetes-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.
I may be wrong but are we not removing thank-you pages and replacing them with the #success tags.
Like this one https://ubuntu-com-14660.demos.haus/kubernetes/compare#success
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.
It actually routes to the banner as is but indeed probably better to explicit 🙂
templates/kubernetes/compare.html
Outdated
<div class="row--50-50"> | ||
<hr class="p-rule--muted" /> | ||
<div class="col"> | ||
<h2>Multi-cloud deployments & operations</h2> |
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.
To be changed according to copydoc
<h2>Multi-cloud deployments & operations</h2> | |
<h2>Multi-cloud deployments and operations</h2> |
Thanks @immortalcodes! Could you have another look? I applied all changes except the one suggested in this first screenshot as anchoring the text with a col-6 hr was requested during design review |
LGTM! Nice work 🎆 |
b2bf078
into
canonical:k8s-bubble-refresh
Done
QA
Issue / Card
Fixes https://warthogs.atlassian.net/browse/WD-12877