search

nginx / nginx.org

Sources for the NGINX website and documentation
https://nginx.org
Other
48 stars 18 forks source link

General usability enhancements. #20

Closed lcrilly closed 2 months ago

lcrilly commented 4 months ago

This is the first of a multiple patch series for nginx.org.

This changeset brings numerous usability enhancements while retaining the familiar look and feel of the site.

The primary visual changes are:

*) Readability on mobile devices

*) Increased contrast and emphasis for code and configuration styles in the 'pre' and 'code' blocks.

*) Dark mode honors the client's system preferences and is automatically applied.

The embedded stylesheet has been extracted into separate files to aid ongoing development and maintenance. The special character layouts for the redundant Hebrew and Chinese translations are preserved but have not received further enhancements. The Russian and English stylesheets are currently identical and will be consolidated in a future changeset. For the timebeing, any changes to one must be copied to the other.

This changeset should apply cleanly without requiring configuration changes to the underlying web server.

Commits:

y82 commented 3 months ago

Links to previous round of review in mailman, may be handy for reviewers:

Stylesheet style changes. https://mailman.nginx.org/pipermail/nginx-devel/2023-August/3AYJRIHG6CUV5J3SZM5HWNNCY3572LJZ.html

Responsive menu. https://mailman.nginx.org/pipermail/nginx-devel/2023-August/YWK2JW6PRZVZ4JEYG7ZVGP3T4JPUNEKP.html

CSS as file. https://mailman.nginx.org/pipermail/nginx-devel/2023-August/ZZJUFW5CBCC766DUPVBIFJERXRN3XE4A.html

oxpa commented 3 months ago

It's OK from "internal machinery" standpoint of view. It should work well. But I have some comments for the style changes: The major one:

Two minor points:

lcrilly commented 3 months ago

@oxpa many thanks for the review.

  • config snippets (the <pre> tags) are now very similar to directive headers (div.directive). Either one of them should be different.

I see what you mean, but in what way is this problematic for you? Are you looking for a clear visual distinction between each directive (and so the snippets are confusing)? Or is their similarity confusing in itself? error_page might be a good example to explain further.

  • "hamburger" button can be "misclicked" between the horizontal lines. I did this several times before realizing what's going on. It's worth adding a background element which will be a button instead of the three lines.
  • The menu itself can't be hidden with a click outside the menu. A background element covering the whole screen may help here.

@TasoOneAsia could you please suggest a solution for these as part of your review?

lcrilly commented 2 months ago

Then I might start by submitting a precursor PR that makes the banner part of the site layout "proper". Then the hamburger will always have somewhere safe to live.

This has been sitting for a while. I did look into converting the banner to an XML resource instead of a post-load event but it is not an immediate solution to the menu mis-click issue.

In fact the mis-click reported by @oxpa (admittedly minor) does not affect touchscreen devices which is where it will show up most of the time. @TasoOneAsia confirmed that it is not reproducible on Android or iOS devices. So I consider this the subject of a minor enhancement in the future.

@oxpa and @TasoOneAsia I will re-request your review/approval now.

y82 commented 2 months ago

just a note that the last forcepush 96a4dd2 is the merge of review (Review fixes and Docker: corrected css sync target, forced xslt rebuild.) to the main patches