-
Notifications
You must be signed in to change notification settings - Fork 43
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
Change README to markdown #101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some quite significant issues with the html that has been introduced here that need to be addressed. Most pertinent is whether we need this html at all and could not just use pure markdown and achieve what arguably would be nicer than the result proposed here.
@@ -0,0 +1,74 @@ | |||
<h1 align="center" style="margin:1em;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must the first elements of the readme be centre-aligned? I don't see there was any problem with everything being left-aligned as in the original. Centre-aligning the title and first section makes for an awkward disconnect when we get to the first section that isn't centre-aligned.
Whether or not the first elements should be centre-aligned or not, it's bad form to mix styling and content as is being done here.
@@ -0,0 +1,74 @@ | |||
<h1 align="center" style="margin:1em;"> | |||
<a href="https://iris-grib.readthedocs.io/en/latest/">iris-grib</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you making the readme title an obfuscated link? If we want a link to the docs in the README, we should add it explicitly below.
<a href="https://iris-grib.readthedocs.io/en/latest/">iris-grib</a> | ||
</h1> | ||
|
||
<h4 align="center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having non-contiguous heading levels (that is, an h3 tag should never directly follow an h1 tag etc., but can follow another h3, or lower level tag) is bad form as it reduces web page accessibility for tools such as screen readers.
</p> | ||
<br> | ||
|
||
# Table of contents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, a webpage should never have more than one h1 tag. Please make this an h2 tag.
|
||
The full text of the licence can be found in the COPYING and COPYING.LESSER | ||
files. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Down here would be an ideal place to add a section with a link to the docs rather than having the readme title doubling as a docs link.
|
||
<!-- tocstop --> | ||
|
||
## Code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you drop the heading level of the above section you should consider whether this heading level also needs to be dropped.
The full text of the licence can be found in the COPYING and COPYING.LESSER | ||
files. | ||
|
||
## License and copyright |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again.
GRIB interface for <a href="https://github.com/SciTools/iris">Iris</a>. | ||
</h4> | ||
|
||
<p align="center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A p
tag is a paragraph tag, and should contain a paragraph's worth of text. If you wish to wrap all the shields below in a single html element to centre-align them you should be using a div
tag.
<img src="https://zenodo.org/badge/5282596.svg" | ||
alt="zenodo" /></a> --> | ||
</p> | ||
<br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The br
tag is self-closing, so should be rendered as <br />
. In fact, I don't see that it is needed at all, so perhaps you could remove this line.
Hi @dkillick all of the formats you've commented on were directly copied from the current cf_units README.md. These changes were introduced in Markdown readme 103. The intention of @pelson 's change was to make the cf_units more like the proposed Cartopy README and trying to start standardising scitools READMEs. Would we be able to address all of these at once? So we agree on the changes first and then we can make them to cf_units and Cartopy too. |
@LukeC92 apologies for roasting you for prior art! 💥 😢 I only realised after I'd submitted the review that you'd made use of an approach introduced elsewhere... My comments on mixing style and content / webpage accessibility still stand though (I think). Could you check what the current web guidelines are for styling / accessibility and suggest some changes to this document that as far as possible maintain its rendered appearance while doing your best to also adhere to the current web guidelines? Thanks 👍 |
It’s been difficult to find advice that speficically relates to this Pull Request. The 2 pages from w3schools offer some useful hints for design and accessibility. For example heading tags should be used to show the structure of a page. Semantic tags e.g. “button”, “paragraph” etc should be used when their name is relevant. The second link mentions how empty tags should be closed. These pages offer some good general advice which isn’t strictly applicable in this situation. Regarding Accessibility there are a lot of good blogs and articles available. For the most part they offer “general” advice rather than “You’re HTML should be organised like this and use these techniques.” One such page is linked to below. Many of the pages I found offer similar advice and often seem to be built on the Web Content Accessibility Guidelines (WCAG) which I’ve discussed further below. One piece of advice that seems pertinent, is having the code for navigation menus at the bottom of the page, so screen readers aren’t stuck reading them first. There’s also the need to not rely too heavily on colour. It seems the standard for web Accessibility is the Web Accessibility Initiative (WAI) from World Wide Web Consortium (W3C). Most of the pages I found made a few references to WAI. The Web Content Accessibility Guidelines (WCAG) 2.1 from WAI is specifically referred to a lot. W3C go into a lot of detail regarding these Guidelines and they can be engaged with in a very broad or a very specific perspective. Below are 3 links to the most official and detailed information available regarding WCAG 2.1. Below those links I’ve tried to cut WCAG down to its key points and sub-points. The Guidelines (things to focus on) are below. Note w3c go into much more detail with each sub-point having between 1 and 13 sub-sub-points:
|
Thanks @LukeC92 The effort on this PR appears to have gone stale, so I'm going to close it for now. Please do re-open it if you want to push it forwards. |
Using the change to the cf_units as a model I've changed the iris-grib README.rst to README.md. The text content is pretty much identical to how it was before, the main changes are formatting. Below is a list of the content I changed: