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

Implement base JS + responsive menu #507

Merged
merged 19 commits into from Sep 1, 2020

Conversation

samueldr
Copy link
Member

The responsive menu has been implemented as an example of a good citizen JavaScript component. Though it's not the best component to show as an example :/.

The main scope of the work here has been:

  • Cutting the old cruft
  • Updating jQuery (yes, we're sticking with that, fancy frontend libraries lovers)

While doing that, the setup for in-line JavaScript (and other tags) has been reviewed. See the example in index.tt, where atEnd and atHead have been used. This allows the page author to correctly add the tags where they belong.


Responsive menu

Let me explain the approach I used to implement it.

The first step is to figure out the DOM and mechanics to make it usable without JavaScript. The DOM I mostly already knew when I implemented the navbar. Simply put, don't repeat the menu uselessly, simply have it as part of the header. Don't add elements in the DOM that are not usable when JavaScript is not enabled!

So I knew that I needed the menu to be hidden by default, or else you'll see it temporarily on load, while the JavaScript hides it quickly. Not good. The solution is to have the DOM hide it, but on html.without-js, you force it open using !important. The without-js classname is changed to with-js ASAP for this purpose, and other similar purposes.

Now, this looks dirty, going with !important that way, but deal with it, it gracefully degrades quite well using this scheme.

The !important trick is also used to force the menu to be shown when the viewport is wide. This removes all guesswork with resizing the window and showing or hiding the menu. Simply put, it automatically degrades (or upgrades?) to the proper menu, whether it was hidden or not using the hamburglar menu icon.

Finally, the scheme was to use jQuery.slideToggle(). So this is what is used on the button. Note that the button is added with JavaScript. This makes usre the JavaScript-less page does not show a useless element.

In addition to removing the now unused pager block.

The added blocks collect the HTML used within them, which will in turn
be inserted at the proper location.

This mostly serves my need for everything to be tidy, but it also serves
the need for everything to be *correct*.
This allows the developer to make CSS rules to improve the behaviour of
the site when JavaScript is not enabled.
@github-actions
Copy link
Contributor

@github-actions
Copy link
Contributor

@jonringer
Copy link
Contributor

not related to the PR, but just want to say thanks to @samueldr and @garbas , for giving nixos a new look. It's amazing what some UX does to make it seem that much more appealing.

Copy link
Member

@davidak davidak left a comment

Choose a reason for hiding this comment

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

i'm not qualified to review the JS and CSS in detail. i have not seen an obvious error

the menu works as it should. small design improvements needed

@davidak
Copy link
Member

davidak commented Aug 28, 2020

i guess the border in the burger menu should be removed

the text can be bigger. remember, it is supposed to work on tiny smartphones

Screenshot from 2020-08-28 15-44-09

the download and get startet button can still be in the same row

@samueldr
Copy link
Member Author

The border around the items is as-designed. I am implementing it as-designed, we can see for further bike shed paint jobs after the whole implementation. Otherwise we'll never move forward.

Similar point for the font size. I need to configure a "normal" phone font size and confirm whether it is fine or not. (My phone is not configured "normally", font-size wise, so things can end up weird.) Though if it isn't okay, the whole site's font size (on narrow viewports) will be increased. Targeting the menu only is probably a mistake :).


the download and get startet button can still be in the same row

Actually they can't :) (But this is not related to this PR).

To improve reachability of the buttons, they are by design put in a vertical layout on narrow designs. This way we are not assuming the whole screen can be reached, but rather that a line or an arc is.

(Though, the hamburglar menu doesn't really reflect this, and can't really trivially.)

@samueldr
Copy link
Member Author

samueldr commented Aug 28, 2020

Just verified on device (Android), it does respect the user's "display size" setting, as conceived, and on "Default" is just fine, compared to apps and other sites. The font is "too big" when directly comparing to apps, which means it's just fine as designed.

Note that your screenshot shows a width that is quite too wide compared to what a phone would use. The best way to test mobile scale is to use the inspector feature of your browser, and use its specific feature for "Responsive Design Mode" (firefox) or "device toolbar" toggle (chrome).

@github-actions
Copy link
Contributor

@samueldr samueldr added the design About the design refresh work label Aug 28, 2020
@davidak
Copy link
Member

davidak commented Aug 29, 2020

Using Chromium. When selecting iPad it looks like this:

Screenshot from 2020-08-29 11-36-44

and here is a lot of white space

Screenshot from 2020-08-29 11-35-51

with galaxy fold:

Screenshot from 2020-08-29 11-37-17

@samueldr
Copy link
Member Author

(I'll comment / test later for the menu issues)

The white space you are seeing is not relevant to this PR, it is due to the lack of graphical element for the incomplete front page styling. Assume the main page styles are incomplete.

The 0.001 wasn't thoroughly tested. It turns out that with chrome it
ends up activating on the 768 breakpoint even though it shouldn't have.

At 0.01 it also triggers.

This is now set back to a tenth of a pixel.
This made the gutter "carving" not happen, instead the maths as
made by LESS resulted in 47.8rem instead of 46rem.
The layout is tightly controlled in that orientation. Let the user
scroll instead.

This works around *parts* of a problem with way-too-narrow for comfort
sizes like the purported Galaxy Fold's 280 device pixel width for, what
I presume is, the external display.
Yes, we're scaling down the site, but we are **not** making it worse.
This is because the site structure will not be glitched out by the weird
proportions of those devices, and the users can still zoom-in.

In fact, the actual pixel density of their displays allows the user to
simply bring the phone closer to their face also!
@github-actions
Copy link
Contributor

@samueldr
Copy link
Member Author

samueldr commented Aug 30, 2020

The iPad-size issue is a bad assumption at chrome's behaviour with responsive breakpoints.


Whew, that way-too-narrow viewport issue is an annoying one!

First, the "Galaxy Fold" screen is for the outside screen. AFAICT it's a pain in the side of web developers since it's way too narrow according to its defaults.

Now, chrome "helpfully" zooms out of the page to fit the big words in the available width. So the navbar isn't actually less wide than the page width, it's actually fitting snugly against the viewport's sides at 1.0 zoom factor! (As can be observed in firefox with the width set to 240). As a first part of working around this issue, the zooming out that way is now disabled. This is not an accessibility issue like zooming in, since zooming out is something mobile browsers don't generally allow you to do anyway.

Now, the other part of the solution is to take any width smaller than the iPhone 5 / SE and scale the site down relative to the viewport width. You'll think "OH NO, THE TEXT WILL BE TOO TINY". Except that our text was already "too big". It is common for sites to design at a computed font size of 16px-equivalent. With the automatic scaling down, the font coincidentally ends up being computed to 14px-equivalent at the Galaxy Fold breakpoint. Lucky!

image

image

@samueldr samueldr merged commit c88ea50 into feature/2020-redesign Sep 1, 2020
@samueldr samueldr deleted the feature/2020-r/base-js branch September 1, 2020 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design About the design refresh work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants