hero and navbar improvements #129

reveurguy · 2022-06-25T16:50:19Z

Signed-off-by: Priyanshu Singh [email protected]

made changes to hero section and navbar, as part of initial changes. Improved their look. Please provide your feedback or any changes.

estesp · 2022-06-27T14:46:18Z

you need to squash these 2 commits into one since the whitespace and DCO checking happens on each commit in the pull request. Let me know if you need help, but that should get you a clean CI run so we can review.

reveurguy · 2022-06-27T15:52:30Z

I squashed the commits but the previous failed commit is still showing.

reveurguy · 2022-06-27T16:28:36Z

ok, i reset the branch, did a new commit . but after that it's telling me to pull and when i pull, it is merging branches and in that merge that previous failed check commits are present. So it's quite a situation here.

Signed-off-by: Priyanshu Singh <[email protected]>

estesp · 2022-06-27T17:55:53Z

Looks like a nice refresh to some of the UI elements on the main page; anyone else @containerd/committers have an opinion?

dmcgowan · 2022-06-28T23:37:38Z

I agree it is a nice refresh, a few questions/comments

Where does the background blue come from, is it used somewhere else for consistency?
The "Getting Started" button could have more contrast with the background so it pops out a little more
"containerd Branding" or "containerd branding", we always keep references to the project as lower case, even in titles.

reveurguy · 2022-06-29T06:21:48Z

I agree it is a nice refresh, a few questions/comments

Where does the background blue come from, is it used somewhere else for consistency?

The "Getting Started" button could have more contrast with the background so it pops out a little more

"containerd Branding" or "containerd branding", we always keep references to the project as lower case, even in titles.

I have used the background just for header and hero currently, I will apply the background to the footer and features section as well, that would give a nice scroll effect on the section and would be consistent as well. I used blue just to give a dark background with light text on it like the current one where a blackish shade is present. If it doesn't look great, I can try some other dark shades or try to do a vice-versa, a lighter background with dark text.
Ok I will change that to some light color so that it stands out and is clearly visible as well.
I'll change that to lower case.

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy · 2022-06-30T03:43:45Z

I have made the changes I mentioned above.

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy · 2022-07-08T04:59:26Z

Any suggestion or changes ?

dmcgowan · 2022-07-18T22:24:32Z

Need rebase to remove merge commit. Also either remove the npm packages lock or if necessary move under the node.js comment

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy · 2022-07-20T05:06:50Z

removed the package-lock file. Also can you do the rebase while merging the PR. Trying to do that locally can become quite messy.

Signed-off-by: Priyanshu Singh <[email protected]>

chalin

Interesting redesign, but it breaks for narrow views and the contrast for the docs pages isn't right. I'll submit screenshots shortly.

A few questions:

In terms of branding, is this change in line with any branding color scheme?
Has this redesign been requested and/or approved, if necessary, by the governing committee?

IMHO, this PR changes too many things at the same time. I'd suggest breaking it down into smaller changes submitted through separate PRs.

package-lock.json

chalin · 2022-07-21T12:37:36Z

Here are a few screenshots illustrating design issues that appear for narrow views:

Top-nav containerd logo is clipped:
IMHO, the top-nav logo is too big (the previous size seemed better).
IMHO, the CTA buttons are too big (see the screen shot above).
The vertical logo doesn't work for narrow displays:

The social media buttons in the footer are too big (the previous size was ok). They used to fit nicely together in the default view, but they don't anymore

The top-nav color needs to be adjusted for doc pages:

/cc @nate-double-u

chalin · 2022-07-21T13:05:51Z

/cc @thisisobate for insight into design issues

reveurguy · 2022-07-21T14:04:32Z

Hii chalin, i was waiting for the final confirmation for the design and had made changes initially for the laptop size only, that's why the site has some issues at smaller screen size.

thisisobate · 2022-07-21T14:49:19Z

I completely agree with everything you said @chalin
Also, the flipped logo doesn't sit well with the overall design.

I suggest we revert the logo to be horizontally positioned.

I kinda like the new background image; makes it look fresh. My only concern is that it replaces the brand color.

Btw, great work @reveurguy; you got this!

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy · 2022-07-21T16:14:33Z

I have made the changes @chalin mentioned for small screen devices.

chalin · 2022-07-22T15:32:38Z

Thanks @reveurguy, I'll let @thisisobate have a look at the updates when he has the time.

chalin · 2022-07-22T15:34:14Z

@amye and/or @caniszczyk: any comments about the change in colors proposed by this PR, and whether that's an issue in terms of containerd branding, given the current color palette of containerd logos.

/cc @nate-double-u

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy · 2022-08-01T11:27:41Z

Any update on this? @chalin

reveurguy · 2022-10-15T14:32:49Z

Hii, any update on this PR? Do let me know if any changes are required.

chalin · 2022-12-16T20:52:12Z

Thanks for your patience. Again, I'll let @thisisobate follow up on this (he's currently out -- or soon will be -- for the holidays if I'm not mistaken).

thisisobate · 2023-01-11T06:56:40Z

@chalin LGTM! Thanks for your patience! @reveurguy

chalin

Thanks for the updates @reveurguy, and the review @thisisobate.

Good progress

Generally, I like the updates @reveurguy, but making a UI skin change can be challenging to get right. I very much appreciate your efforts, but there is more work to be done. Hopefully you're still up to the challenge!

See below for two other UI issues I've identified. More importantly though, there are two major design problems, as I describe next.

Two major design problems

This website was built using bulma. In this PR you've added the import of Bootstrap and it's Popper helper (see inline comment in navbar.html). We don't want that. CSS frameworks are resource heavy, and add a significant overhead for maintainers (even if you're only importing the JS resources). Use only either bulma or Bootstrap, not both. I'd love to see this project upgraded to use Bootstrap, but that should be done (if at all) after these UI skin changes land.
You've hardcoded styles through use of the style attribute. Styles should be added via classes defined by the CSS framework, or custom classes defined in the SASS style file.

Also see inline comments for other issues.

Remaining UI issues under mobile

Under mobile, the hamburger's color contrast isn't enough as you can see here (in the red circle), and in the next image:

The contrast between the logo (in gray) and the top-nav background (in dark gray), could be improved too.

Also, the drop-down menu from the hamburger needs font color fixes too:

Thanks!

chalin · 2023-02-04T14:57:42Z

Makefile

-check-links: clean production-build install-link-checker run-link-checker
+check-links: clean production-build install-link-checker run-link-checker


I think that nothing was changed here other than whitespace. Please undo the whitespace change.

chalin · 2023-02-04T15:04:31Z

themes/containerd/layouts/partials/footer.html

          Docs
        </span>

-        <hr class="hr has-background-grey">
+        <hr class="hr  ">


Nit: remove unnecessary space in class string, here and the other instances below.

chalin · 2023-02-04T15:18:09Z

themes/containerd/layouts/partials/navbar.html

+<script src="https://code.jquery.com/jquery-3.5.1.slim.min.js" integrity="sha384-DfXdz2htPH0lsSSs5nCTpuj/zy4C+OGpamoFVy38MVBnE+IbbVYUew+OrCXaRkfj" crossorigin="anonymous"></script>
+    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/umd/popper.min.js" integrity="sha384-Q6E9RHvbIyZFJoft+2mJbHaEWldlvI9IOYy5n3zV9zzTtmI3UksdQRVvoxMfooAo" crossorigin="anonymous"></script>
+    <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.5.0/js/bootstrap.min.js" integrity="sha384-OgVRvuATP1z7JjHLkuOU7Xw704+h835Lr+6QL9UvYjZE3Ipu6Tp75j7Bh/kR0JKI" crossorigin="anonymous"></script>


CSS frameworks are resource heavy, and add a non-trivial overhead for maintainers. This website was built using bulma. Here you're adding full support for Bootstrap. We don't want that. Use either bulma or Bootstrap, not both.

chalin · 2023-02-04T15:28:13Z

themes/containerd/layouts/partials/navbar.html

@@ -96,3 +96,16 @@
    </div> <!-- .navbar-menu -->
  </div> <!-- .container -->
 </div> <!-- .navbar -->
+
+<script src="https://code.jquery.com/jquery-3.5.1.slim.min.js" integrity="sha384-DfXdz2htPH0lsSSs5nCTpuj/zy4C+OGpamoFVy38MVBnE+IbbVYUew+OrCXaRkfj" crossorigin="anonymous"></script>


This project already includes jQuery 3.6 (see under the themes folder).

chalin · 2023-02-04T15:30:19Z

themes/containerd/layouts/partials/navbar.html

+<script>
+        $(function () {
+            $(document).scroll(function () {
+                var $nav = $("#mainnav");
+                $nav.toggleClass("scrolled", $(this).scrollTop() > $nav.height());
+            });
+        });
+    </script>


navbar.html shouldn't contain embedded script elements. Instead add the code in a manner consistent with this project (see the javascript.html partial).

chalin · 2023-02-04T15:31:00Z

themes/containerd/layouts/partials/footer.html

+      <div class="column" style="display: flex;justify-content: center;align-items: center;">
+        <img style=" transform:scale(1.2);" src="{{ $logo }}" class="is-footer-logo" alt="containerd footer logo">


Don't hardcode style attributes.

chalin · 2023-02-04T16:22:43Z

Btw, when working on the burger contrast issue, consider increasing the height of .navbar-burger span to 2px. That'll help make it more visible.

hero and navbar changes

25a2578

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy force-pushed the main branch from c5472a6 to 25a2578 Compare June 27, 2022 17:02

reveurguy added 2 commits June 30, 2022 07:43

Merge branch 'containerd:main' into main

bc19a15

changed background of footer and features-section

3973c3f

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy force-pushed the main branch from 67c9963 to 3973c3f Compare June 30, 2022 03:34

updated bulma version

d97eb62

Signed-off-by: Priyanshu Singh <[email protected]>

removed package-lock file from gitignore

d4ce849

Signed-off-by: Priyanshu Singh <[email protected]>

reveurguy added 4 commits July 20, 2022 13:44

hero and navbar changes

bebfa51

Signed-off-by: Priyanshu Singh <[email protected]>

changed background of footer and features-section

f3f8013

Signed-off-by: Priyanshu Singh <[email protected]>

updated bulma version

e6c9a07

Signed-off-by: Priyanshu Singh <[email protected]>

removed package-lock file from gitignore

496b90b

Signed-off-by: Priyanshu Singh <[email protected]>

dmcgowan force-pushed the main branch from d4ce849 to 496b90b Compare July 20, 2022 20:45

dmcgowan approved these changes Jul 20, 2022

View reviewed changes

chalin suggested changes Jul 21, 2022

View reviewed changes

package-lock.json Outdated Show resolved Hide resolved

reveurguy added 2 commits July 21, 2022 16:09

made changes for small screen size according to suggestions

a8552f5

Signed-off-by: Priyanshu Singh <[email protected]>

made changes for small screen size according to suggestions

658ce98

Signed-off-by: Priyanshu Singh <[email protected]>

removed package-lock

9225016

Signed-off-by: Priyanshu Singh <[email protected]>

Merge branch 'containerd:main' into main

74b420d

chalin suggested changes Feb 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hero and navbar improvements #129

hero and navbar improvements #129

reveurguy commented Jun 25, 2022

estesp commented Jun 27, 2022

reveurguy commented Jun 27, 2022

reveurguy commented Jun 27, 2022

estesp commented Jun 27, 2022

dmcgowan commented Jun 28, 2022

reveurguy commented Jun 29, 2022

reveurguy commented Jun 30, 2022

reveurguy commented Jul 8, 2022

dmcgowan commented Jul 18, 2022

reveurguy commented Jul 20, 2022

chalin left a comment

chalin commented Jul 21, 2022

chalin commented Jul 21, 2022

reveurguy commented Jul 21, 2022

thisisobate commented Jul 21, 2022

reveurguy commented Jul 21, 2022

chalin commented Jul 22, 2022

chalin commented Jul 22, 2022

reveurguy commented Aug 1, 2022 •

edited

Loading

reveurguy commented Oct 15, 2022

chalin commented Dec 16, 2022

thisisobate commented Jan 11, 2023

chalin left a comment

chalin Feb 4, 2023

chalin Feb 4, 2023

chalin Feb 4, 2023

chalin Feb 4, 2023

chalin Feb 4, 2023

chalin Feb 4, 2023

chalin commented Feb 4, 2023

		check-links: clean production-build install-link-checker run-link-checker
		check-links: clean production-build install-link-checker run-link-checker

		<div class="column" style="display: flex;justify-content: center;align-items: center;">
		<img style=" transform:scale(1.2);" src="{{ $logo }}" class="is-footer-logo" alt="containerd footer logo">

hero and navbar improvements #129

Are you sure you want to change the base?

hero and navbar improvements #129

Conversation

reveurguy commented Jun 25, 2022

estesp commented Jun 27, 2022

reveurguy commented Jun 27, 2022

reveurguy commented Jun 27, 2022

estesp commented Jun 27, 2022

dmcgowan commented Jun 28, 2022

reveurguy commented Jun 29, 2022

reveurguy commented Jun 30, 2022

reveurguy commented Jul 8, 2022

dmcgowan commented Jul 18, 2022

reveurguy commented Jul 20, 2022

chalin left a comment

Choose a reason for hiding this comment

chalin commented Jul 21, 2022

chalin commented Jul 21, 2022

reveurguy commented Jul 21, 2022

thisisobate commented Jul 21, 2022

reveurguy commented Jul 21, 2022

chalin commented Jul 22, 2022

chalin commented Jul 22, 2022

reveurguy commented Aug 1, 2022 • edited Loading

reveurguy commented Oct 15, 2022

chalin commented Dec 16, 2022

thisisobate commented Jan 11, 2023

chalin left a comment

Choose a reason for hiding this comment

Good progress

Two major design problems

Remaining UI issues under mobile

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin Feb 4, 2023

Choose a reason for hiding this comment

chalin commented Feb 4, 2023

reveurguy commented Aug 1, 2022 •

edited

Loading