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

New navbar. #177

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

New navbar. #177

wants to merge 6 commits into from

Conversation

raven2cz
Copy link
Contributor

@raven2cz raven2cz commented Oct 1, 2022

@actionless According to our discussion - navbar and css improvements issue #159.

There is new prototype 4. With complete new design for navbar and full support for mobile devices (overlay menu support). Fixes for few css "gaps" and small improvements according to linked pags in #159.

From my side, it is better design.

But of course it takes some screen space, the navigation is still available, it has cons and pros.

https://raven2cz.github.io/awesome-www/

@actionless
Copy link
Member

i like the overall style a lot, but could we try couple of slightly darker variations for the starting gradient color on the very top?

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 1, 2022

@actionless There are proposals. Choose your "destiny"

@actionless
Copy link
Member

they all still lack a little bit of contrast:

from light i didn't liked much neither, they still got too light top color

from dark dark-5 and dark-1, but again, i'd like top menu color still to be lighter

@actionless
Copy link
Member

and i see you're adding new font - make sure that all text then would be using the new font family, it would be like looking tidier (you could additionally play with font-weights)

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 1, 2022

@actionless Maybe I have idea...

navbar-design-B.tar.gz

@actionless
Copy link
Member

actionless commented Oct 1, 2022

previous i liked more, design was good, just top-most color should be like 5-10% more contrast

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 1, 2022

I have no directly contrast parameter in color, but I can change saturation which I increase about 10%. In addition there is necessary to play with alpha channel. There are two images with increased saturation. C1 image has alpha channel 30, more opacity (more original color). C2 has alpha channel to 20, less opacity (less original color).

dark-5-C.tar.gz

@actionless
Copy link
Member

mb smth in the middle of these two? 😸

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 2, 2022

mb smth in the middle of these two? smile_cat

Demo updated. Font family used just one.

Main problem was light theme, I tried around 40 settings and this is best which I found. For light theme is maybe to remove gradient for light or kept this one.

@actionless
Copy link
Member

previous gradient color on light theme was good, it just need to be darker, not different color, just darker and that's it

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 2, 2022

Increase darker just few percent in value and 2 variants of alpha color.

light-2-B-darker-.tar.gz

@actionless
Copy link
Member

actionless commented Oct 2, 2022

i think light-2-B-darker-60 is alright, but looking on it i want to move menu itself (and light/dark switch) like 10 or so pixels higher (without moving the gradient itself)

@actionless
Copy link
Member

and could we also get rid of using underline for currently selected menu item, please, and think of smth different for that, like for example using different color and (background color or gradient) for the selected menu item

@actionless
Copy link
Member

also a technical problem - with certain window width github link is overlapping the "home" link and attempts to click Home open github instead:

2022-10-02--1664708738_1920x1080_scrot

@actionless
Copy link
Member

i'm not 100% sure, but mb that could be solved by adjusting z-indexes for github strip and menu

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 2, 2022

Done.

About correction. Z-order is already used, because "fork me" needs highest z-order. I have just one solution, to switch to "menu" button only if the width is less than nav-links buttons. I tested now in browsers and mobiles. It seems ok now.

All next points are done. Selected menu item too. About possibility to change color-background. We will see. I used color and width of text first. Color-background is very dominant change in whole design.

@actionless
Copy link
Member

actionless commented Oct 2, 2022

  1. now the menu get hidden into button when the window is quite wide - that normally should happen only on obviously-mobile widths - even in previous commits of this prototype that was happening too early, but now it's even more

  2. when scrolling down menu shade totally disappearing, this looking a bit inconsistent, i think it should still persist, mb not as gradient but as a lighter, solid color:
    2022-10-02--1664716727_1206x270_scrot
    2022-10-02--1664716505_1205x216_scrot

@actionless
Copy link
Member

actionless commented Oct 2, 2022

  1. position of dark/light button should be consistently in top right corner when scrolling
  2. same for menu itself - after doing pt.3 that shouldn't be hard

@actionless
Copy link
Member

  1. it feels very weird to have site logo below the menu, i recommend moving it permanently to the left (like on scrolled-down menu layout) for all the pages except for Home page (there showing it below menu sorta works as in such case logo looks like part of page layout, not header)

@actionless
Copy link
Member

actionless commented Oct 2, 2022

and also i'm not sure about github ribbon - back in the day when it was added it was like a sign of project being modern and instead of using self-hosted svn using public git repo with collaboration possibilities

nowadays it's sorta a standard already, so i don't really think it's still reasonable to additionally advertise the fact we use github, what do you think @Elv13 ?

the reason i wanna get rid of it, because it makes the header layout possibilities more complicated

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 2, 2022

Uch. All points will be much more bigger problems now.

Add 1) It is my correction to your reported bug. "Fork me" needs highest z-order, because it has to be independent on the navbar. If it has lower z-order, it cannot be press. There is just one choice, to replace nav-links with menu button - if the width of buttons is smaller and to close to "fork me". If we want "Fork me" in the left side, we haven't other choice. Another solution is completely remove "Fork me" from top segment.

Add 2) It is my design. I don't want gradient in the scrolling menu. Do you want it? This navbar is much more smaller and has different behavior. I think that it is consistent. If we will apply some gradient, it needs different configuration for it, because the it is rolling navigation bar - it means different feature.

Add 3) It is my design again. The animation for left logo and right dark/light button has to be consistent. If there will be just animation for left logo, it is asymmetric animation and very bad for eye. I already play with it and it is not good idea.

Add 4) It is very big problem. It change whole implementation. Because I cannot use simply flexbox which measure space between components. I haven't technical solution for this requirement. I can find some solution for it, but it will change whole implementation.... Problem here is that flex measure symmetric spaces between inserted elements. And there is now visible left logo, the space between logo, nav-links and right button has need same space-between. You want some asymmetric space, which cannot be fixed, because you have dynamic width of window.
Maybe to drop whole flex and use some center in left, center and right parts....I haven't idea now for this point.

Add 5) Menu is independent on the page content. You want cross concern between ikiwiki template and menu component. I cannot make this change. Template has to be simple, I have bound hands with ikiwiki. The change has to be consistent for all pages, not just some home or something.

The complete removing "fork me" button very help.

I'm not sure with this Draft MR now. It seems that there is much more problems against previous design. So, I cannot make all these points. Check my responses. Some of them is possible to solve. Some can take much more time. And some is not possible to solve.

@actionless
Copy link
Member

I don't want gradient in the scrolling menu. Do you want it?

i think it should still persist, mb not as gradient but as a lighter, solid color

regarding the rest - yup, smth like space-between or few different media-queries for different screen width would help there

but let's first get some feedback regarding Fork ribbon - as it would make things easier to plan if we can make decision to leave it or get rid of it - and so next we could discuss everything more detailed from that point

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 2, 2022

Yes, we will wait.

In addition, think about possibility to remove dynamic "rolling" nav. Maybe it is a luxury which we don't need because we have very small content in each page. If we remove dynamic nav. We have free hands with more variants for other pages against home. I play just ideas.

There is just some concept of top bar very similar with actual style.
image

@actionless
Copy link
Member

visually i like that thing with subtle color and/or gradient - i think it's good visual direction to move, that thing on last screenshot looks like default theme of old Bootstrap version from like 2010

@actionless
Copy link
Member

so do we still need that github ribbon @Elv13 ? (#177 (comment))

Copy link

@sclu1034 sclu1034 left a comment

Choose a reason for hiding this comment

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

  1. I don't like the gradient, personally.
    I'd prefer either no background at all in the "not scrolled" state, or the same background+border as when the page is scrolled

    i think it's good visual direction to move

    I don't think anyone will ever need a visual reminder that a webpage can be scrolled.

  2. The "menu" button looks completely out of place and counterintuitive to the rest of the minimalistic style of the navbar. It's neither consistent with the other menu entries, nor with the other button on the page.
    I'd prefer the standard "hamburger" icon here, also because that has become the expected icon for the purpose.

  3. In the "scolled" state, the navbar entries are off-center (they are centered between the logo and the switch, which have different sizes.
    They should be centered globally instead. That would also prevent them from jumping horizontally between the two states

    It is very big problem. It change whole implementation.

    It's hardly a "very big" problem. You just need a 2-layer layout: An outer flexbox that creates three evenly distributed containers, and then appropriate layouting in each container, such as in this quick sketch.

  4. I'd be fine with removing the GitHub ribbon, and instead changing the "Bugs/Issues" menu entry to just "GitHub" or "Source Code".

  5. Since the buttons to show and hide the menu on smaller displays is located in the top right, the overlay should slide in from either the top or right, i.e. from an edge connected to that button. And the "close" button should be in the same position on the screen as the "open" button.

  6. The order in HTML should reflect the order on screen, so #navbar should come before #pageheader.

  7. On certain screen sizes (e.g. the "iPhone 11 Pro" preset provided by Firefox), the menu button is partially off-screen.

  8. On certain screen sizes (e.g. the "iPhone 11 Pro" preset provided by Firefox), the contents of the slide-in menu are not centered.

  9. On touch devices (or Firefox's "touch emulation"), the navbar does not stay on screen.

  10. The naming style for CSS classes is very inconsistent. Sometimes you use BEM, sometimes you don't. And it's all mixed together.
    I'd be fine with committing to BEM, but it should be consistent and applied correctly.

  11. There is a small interval around a viewport width of 1100 px where the navbar has already been converted to the "desktop" variant, even though it is not yet big enough to fit the entire content, and the right side is cut off.

  12. When scrolling down, the logo is animated to slide into its position. When scrolling up, it simply vanishes.
    Instead, it should have the reverse animation, just like the theme switch.

.top-gradient {
background-image:
linear-gradient(to bottom, var(--background-gradient-color), transparent);
background-size: 100% 2%;
Copy link

Choose a reason for hiding this comment

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

With the background size being determined by page length, the gradient will look different on pretty much every page.

@@ -292,6 +306,7 @@ footer p {
-webkit-align-self: center;
-ms-flex-item-align: center;
align-self: center;
font-family: "Montserrat", sans-serif;
Copy link

Choose a reason for hiding this comment

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

Yet another hardcoded font? We already determined in the last PR that we don't want that.

Comment on lines +342 to +347
.active-menu-about a.menu-about,
.active-menu-download a.menu-download,
.active-menu-community a.menu-community,
.active-menu-recipes a.menu-recipes,
.active-menu-screenshots a.menu-screenshots,
.active-menu-doc a.menu-doc {
Copy link

@sclu1034 sclu1034 Oct 8, 2022

Choose a reason for hiding this comment

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

The common way to do this is to apply an "active" class to the <a> element itself, rather than defining a bunch of classes to apply to the parent. That would make the selector much simpler, and future-proof, as no specific names would be involved.

<a class="menu-issues" alt="bugs/issues" href="https://github.com/awesomeWM/awesome/issues">bugs/issues</a>
</header>
<header id="navbar">
<a class="logo" href="/">
Copy link

Choose a reason for hiding this comment

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

This should point to the same URL as the "Home" menu entry. Currently, it does not respect the base URL.

Comment on lines +61 to +66
<figure class="darkmode zero-margins">
<img id="nav-logo-dark" alt="awesome-logo" src="<TMPL_VAR BASEURL>images/awesome-dark-1.svg">
</figure>
<figure class="lightmode zero-margins">
<img id="nav-logo-light" alt="awesome-logo" src="<TMPL_VAR BASEURL>images/awesome2.svg">
</figure>
Copy link

Choose a reason for hiding this comment

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

A <figure> specifies self-contained content (e.g. a diagram or a photo), usually within a body of text and accompanied by a <figcaption>.
It doesn't make much sense for a logo.

Comment on lines +147 to 149
<script defer src="<TMPL_VAR BASEURL>script/navscroll.js"></script>
<script defer src="<TMPL_VAR BASEURL>script/mobile.js"></script>
<script defer src="<TMPL_VAR BASEURL>script/dark-light-mode-switch.js"></script>
Copy link

@sclu1034 sclu1034 Oct 8, 2022

Choose a reason for hiding this comment

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

As far as I can tell, these do not need separate encapsulation and can all be combined into a single script. The fact that they are separate files just adds overhead.

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 8, 2022

@sclu1034 Many thanks for your technical overview. It is very helpful! I will implemented it step by step, maybe I will have some additional questions, but in common, it is clear.

Before implementation. We will wait with ribbon to @Elv13 opinion too.

But I have two important questions to you that were not answered. I know that you mainly focused to technical part. If this draft / idea is good approach or not. Mainly two points we have to discuss

  • Top menu is present "permanently". It was aim, but for mobiles and small screens it can take important space. Dynamic top menu is good feature, but I don't know if it is accurate for awesome web where is content very small for each tab.
  • @actionless said, that top menu is not ok, because main logo is under it. In addition, you don't want gradient. It means that we have to create some line between main logo and top menu, which exaggerates the problem between top menu and logo again.

I personally like large logo in home page. Next tabs can be without this logo, it will be some inconsistency, but maybe still acceptable. But still top menu is top menu. In home tab, the logo will be still under it.

In addition, if you add the line to fixed menu, there will be very ugly to show animation for menu logo. My main design concept was that the large logo disappears and "in same time" the logo moves to the menu bar with animation to the left. But if we don't want the logo in next tabs, this idea goes to dill.

We really have to discuss what we want.

@sclu1034
Copy link

sclu1034 commented Oct 8, 2022

For the home page, I do like the large logo. But I wouldn't want it on the other pages.

As for navbar above or below the logo, I don't have any strong preference. I'd be fine if it was moved back below the logo, and the animations adjusted accordingly.

It means that we have to create some line between main logo and top menu

I don't see any reason why a line should be considered necessary. The gradient doesn't work as a separator either, so not having the line is exactly what we have right now.

However, simply applying the "scrolled" style even in the "not scrolled" position looks like a valid option to me, too.

Top menu is present "permanently". It was aim, but for mobiles and small screens it can take important space.

I didn't realize that it was actually supposed to reappear while scrolling up. While scrolling down slowly, I saw that the same animation started as on desktop, but that the bar wouldn't stay in place, and assumed that it was a bug.

If the navbar is supposed to disappear on mobile, then it should do so without animation (i.e. simply scroll out of view).
In fact, I think in that case, it would be easiest to just disable the entire "dynamic" portion on mobile and just have it fixed at the top.

However, if its size was reduced, I wouldn't see a problem with the bar staying visible on mobile (i.e. same behaviour as on desktop). Right now, it definitely is too big for mobile screens, and mostly empty anyways.


A few more things I noticed:

  1. Currently, the "scrolled" navbar has a border on all sides, even though three of those sides a flush at the screen edge.
    Instead, the bar should only have a border at the bottom, the only edge where it actually needs to a separator between other content.
  2. Currently, the different navbar styles are applied by assigning multiple CSS properties to the element itself.
    If those two states were instead encoded as CSS classes, and the JS part only swaps between the two classes, that feature would be easier to debug and play around with.

@raven2cz
Copy link
Contributor Author

raven2cz commented Oct 10, 2022

@sclu1034 I have one technical question. From prev conversation, home is selected (it is "about" selection); we want large full logo and menu under it, theme checkbox in right corner. After scrolling, the logo has to disappear and shows the left small logo in left corner. For other non-about tabs, we don't want large logo, but immediately logo in left side, menu middle and checkbox right.

How to solve it? I have idea, that the large logo will be part of your described flex layout, and inserted to middle block, but new style for middle block to use first large logo and menu under it in the middle block. Question is if the hiding in some cases correctly move menu to top if the logo isn't required.

We have this generated "trick" with ikiwiki <TMPL_VAR NAME="title"> which can be used for correct selection of active css style which can be used for styling between about and not: about solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants