Skip to content
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

Build /pro/why-pro page #13182

Merged
merged 4 commits into from
Oct 2, 2023
Merged

Build /pro/why-pro page #13182

merged 4 commits into from
Oct 2, 2023

Conversation

petesfrench
Copy link
Contributor

@petesfrench petesfrench commented Sep 27, 2023

Done

  • Build /why-pro page base on copydoc and design
  • Added some custom styling for the p-divider class to removed a divider from a specific block, issue create here

QA

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-4329

@webteam-app
Copy link

webteam-app commented Sep 27, 2023

Demo starting at https://ubuntu-com-13182.demos.haus/pro/why-pro

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #13182 (4d61623) into main (6860575) will decrease coverage by 0.71%.
Report is 23 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13182      +/-   ##
==========================================
- Coverage   74.45%   73.74%   -0.71%     
==========================================
  Files         106      106              
  Lines        2881     3051     +170     
  Branches      942     1013      +71     
==========================================
+ Hits         2145     2250     +105     
- Misses        712      776      +64     
- Partials       24       25       +1     

see 4 files with indirect coverage changes

@juanruitina
Copy link
Contributor

juanruitina commented Sep 27, 2023

Nice work Pete. Some minor tweaks:

  • Please make the subtitle "All the open source you need..." a p element (styling is correct).
  • There's something on mobile making the search bar at the top dark gray, can you check what's going on?

Captura de pantalla 2023-09-27 a las 17 39 42

  • Also on mobile, the very last link at the end wraps, should stay in full (as in inline-block) at all screen sizes.

Captura de pantalla 2023-09-27 a las 17 39 37

Feel free to change the label to UX+1 once you address this.

@petesfrench
Copy link
Contributor Author

@juanruitina I have addressed the text wrapping issue, this has come about from a recent vanilla change and we should keep an eye out for it elsewhere.

The colour difference in the search also comes from vanilla and is applied when the is-paper class is used and isn't restricted to this page. We should address this with vanilla if we want it changed. The vanilla snippet in question

Comment on lines -11 to -13
{% if schedule_banner("2023-03-08", "2023-03-22") %}
{% include "shared/_charmed-kubeflow-beta-banner.html" %}
{% endif %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carkod Can this be removed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't remove it, we may need it soon in early 2024 again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why do you need to change this page? The ticket was for /pro/why-pro?

@carkod
Copy link
Contributor

carkod commented Sep 29, 2023

@lyubomir-popov can you review? Thank you.

@juanruitina
Copy link
Contributor

The colour difference in the search also comes from vanilla and is applied when the is-paper class is used and isn't restricted to this page. We should address this with vanilla if we want it changed. The vanilla snippet in question

Could we apply is-paper to the wrapper div only? It should do the trick for a bit, and I think this won't be a problem anymore when we go live with the meganav.

@lyubomir-popov
Copy link
Contributor

Looks good, a few small fixes:

  • Hero - can we have shallow padding at the top, deep at the bottom - as it is done here:
    image

Can we have this section like so:
image
(line above headings, not muted, h4 changed to a paragraph, and h4s in the list of 8 items changed to an h5 - with no classes on the h5, just default styling)

u-no-margin--bottom on the small caps heading here:
image

@petesfrench
Copy link
Contributor Author

@lyubomir-popov Can you take another look please

@lyubomir-popov
Copy link
Contributor

Looks good, thanks @petesfrench

@@ -20,7 +17,7 @@ <h1 class="u-sv2">What is Kubeflow?</h1>
<a class="p-button--positive" href="https://charmed-kubeflow.io/docs/install">
Charmed Kubeflow
</a>
<a href="https://charmed-kubeflow.io/#get-in-touch">Contact us now&nbsp;&rsaquo;</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think spaces probably don't have any problems rendering. Just the &rsaquo;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you don't add a &nbsp; it is possible that the &rsaquo; can fall onto the next line on certain screen sizes

Copy link
Contributor

@carkod carkod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I asked Pdms (Lech/Gloria) to take a look, since this is a new pro page, if they don't reply by end of the day you can merge this. Thanks

@carkod
Copy link
Contributor

carkod commented Oct 2, 2023

Good to go! We've got 3 Pdms +1!

@carkod carkod merged commit dd29178 into main Oct 2, 2023
25 of 28 checks passed
@carkod carkod deleted the wd-4329 branch October 2, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants