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

Changes to the page header #7

Merged
merged 1 commit into from Apr 19, 2020
Merged

Changes to the page header #7

merged 1 commit into from Apr 19, 2020

Conversation

@auge8472
Copy link
Contributor

auge8472 commented Apr 15, 2020

First change is to replace the h3 in header with p. There are for me two arguments. First is, that one should not skip heading levels (that would cause a change from h1 followed by h3 to h1 followed by h2) and second (and IMHO more important), the information, stored in the h3, are no headings in itself. We can format the content also in paragraphs eye-catching if wished.


One additional issue. I made testing pages for several contents to be able to understand the desired HTML structure (i.e. loops here and there and the table structure especially of the manager_package_list.html with it's many rowspans and colspans in the table header). Doing so, I came across the menu structure and the order of the main blocks (page header, menu, content). So I played around and came up with a first draft of a menu in a list structure (see therefore this Gist). During the tests I changed the order of the HTML-source from menu=>header=>content to header=>menu=>content. See therefore the creenshot and ignore the "Presented by BaNaNaS".

bananas-menu-1

If I want to change the order also in the code, I have to alter the position of {% block header %}{% endblock %} in the base.html, havn't I (file name should have been order.html)?

Is this desirable at all? If so, I would like to add this small additional change to this PR.

@frosch123
Copy link
Member

frosch123 commented Apr 16, 2020

I don't get why you would want to put the headline over the navigation. That would be like browsers from 10 years ago, when the tabs were below the navigation bar. Somewhen they noticed that was weird, and put the tabs on top, so the URL is inside the tab.
The navigation bar is like the tabs, and the headline is like the URL.

Anyway, the h -> p change in this PR is fine. Do have more to add?
Also, are you aware of this branch: https://github.com/frosch123/bananas-frontend-web/tree/static
It skips the API part and fills the templates with test data.
That should make testing of layouts easy.

@auge8472
Copy link
Contributor Author

auge8472 commented Apr 17, 2020

I don't get why you would want to put the headline over the navigation.

This was only an experiment. I might be a bit oldschool, but because the document has a title (a h1), the nav should have a heading (a h2) and it is (or was) uncommon to place a h2 (of the nav) before the documents title (h1) I reordered the elements for testing. On the other hand it's nowadays not uncommon to make an exception from the above rule for the nav.

So this is all for this PR.


I'll check the static branch.

Copy link
Member

TrueBrain left a comment

I have to say, currently I am much more interested in having this website styled, than <p> vs <h3> following <h1> conversations :D This sounds more like an educational discovery ;)

But as frosch said he is fine with this, who am I to complain :)

@TrueBrain TrueBrain merged commit aee2eed into OpenTTD:master Apr 19, 2020
2 checks passed
2 checks passed
Flake8
Details
Black
Details
@andythenorth
Copy link

andythenorth commented Apr 20, 2020

A typical page should have 2 h1 tags:

  • one titling the site or application
  • one titling the page

This is aimed at AT users and is evidence-based. The evidence has not been updated since 2009, but nor has it been over-turned afaict. https://webaim.org/projects/screenreadersurvey3/#headings

This approach is also consistent with search algorithms and html5 spec.

frosch123 added a commit to frosch123/bananas-frontend-web that referenced this pull request Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.