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

feat: /ceph/install redesign #14511

Merged
merged 5 commits into from
Nov 29, 2024

Conversation

mtruj013
Copy link
Contributor

@mtruj013 mtruj013 commented Nov 27, 2024

Done

  • Applied redesign as per doc and design
  • Drive by:
    • Fixed the djlint toggle setting not placed correctly on /_latest-blogs_download.html

QA

Issue / Card

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

@webteam-app
Copy link

@mtruj013 mtruj013 changed the title Ceph install feat: /ceph/install redesign Nov 27, 2024
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.76%. Comparing base (5a97dae) to head (813021a).
Report is 60 commits behind head on feature-rebrand-ceph.

Additional details and impacted files
@@                   Coverage Diff                    @@
##           feature-rebrand-ceph   #14511      +/-   ##
========================================================
+ Coverage                 69.64%   69.76%   +0.12%     
========================================================
  Files                       120      120              
  Lines                      3419     3426       +7     
  Branches                   1178     1177       -1     
========================================================
+ Hits                       2381     2390       +9     
+ Misses                     1013     1012       -1     
+ Partials                     25       24       -1     

@@ -27,8 +27,8 @@ <h3 class="p-heading--5">
</section>

<script src="{{ versioned_static('js/dist/latest-news.js') }}"></script>
{# djlint:off #}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

djlint will ignore this if it's set within the script

@eliman11
Copy link

eliman11 commented Nov 27, 2024

Thank you! A few comments:

  • Missing description for step 5 under 'multi-node deployment' tab --> 'Here you should see that all the nodes you added have joined the cluster, in the familiar ceph status output.'
  • Change 'containerised' to 'containerized' in the 3rd bullet point under 'containerized' tab
  • Add 'news' to 'Latest news from our blog' heading at the bottom
  • The link in the blog section heading is returning a 404 error - think it's missing /tag. Could you link it to https://ubuntu.com/blog/tag/ceph please? Also edited on the copydoc

@mattea-turic
Copy link
Collaborator

ty @mtruj013 ! just one question from me:

  • there seems to be a lot of wrapping going on here, making the padding pretty deep; could there just be a regular 64px being used?
Screenshot 2024-11-27 at 16 29 53

@mtruj013
Copy link
Contributor Author

Thanks @mattea-turic and @eliman11, ready for another look!

@eliman11
Copy link

Looks good thank you!

@mattea-turic
Copy link
Collaborator

I just noticed the links under the tabs "containerized" and "large-scale" need a hr above it, could you add that in pls? For "large-scale", if you could align them horizontally too like you would as part of a cta block so there aren't too many lines

Screenshot 2024-11-28 at 17 00 48

I'll +1 though :)

Copy link
Contributor

@petesfrench petesfrench left a comment

Choose a reason for hiding this comment

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

LGTM

static/sass/styles.scss Outdated Show resolved Hide resolved
templates/ceph/install.html Outdated Show resolved Hide resolved
templates/ceph/shared/_latest-news-ceph.html Show resolved Hide resolved
templates/ceph/shared/_latest-news-ceph.html Outdated Show resolved Hide resolved
@mtruj013 mtruj013 merged commit 6bc05d1 into canonical:feature-rebrand-ceph Nov 29, 2024
15 checks passed
@mtruj013 mtruj013 deleted the ceph-install branch November 29, 2024 19:20
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.

6 participants