-
Notifications
You must be signed in to change notification settings - Fork 7
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
Initial Semantic HTML change #32
base: master
Are you sure you want to change the base?
Conversation
user_guide/index.html
Outdated
</html> | ||
|
||
|
||
</html> |
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.
Keep the newline at the end.
Pinging @offset-torque Broadly a good idea to me and something I already had in the back of my mind, but removing text-indent is changing a design intent. |
No I don't approve this workflow Frenzie. Someone comes and makes 400+ changes to the guide. How am I supposed to review all these changes and still keep on top of this document. No offense @edent, I am sure you have good intentions but this is not a publicly editable document. I am the author of this guide and you have to tell me verbally before making hundreds of changes to the guide. We have a user guide thread specifically for this but you decided to bypass that it seems. Also our guide is optimized for usability. Removing indents removes all hierarchical paragraph structure and guide now looks like a wall of text. Why ? Also for some reason headings are closer to the upper paragraph. You also changed relative margins into absolute margins. I made everything relative to be able to scale the guide easily. There are no constant pixels in this guide. I don't know what else have you done to this document and why have you done all these things. If you itemize your changes and tell me what are the improvements for each of them, then we can discuss here and decide if they are ok to include. |
I've re-added the indent. Not sure I can add the final newline - my editor strips it off. If you're happy with this, my next PR will remove some of the inline CSS + plus a few other bits of tidying up. |
@offset-torque I'm sorry for overstepping. I didn't see a CONTRIBUTING.md document so I went a bit too far. I appreciate that it is a lot of changes. I'm happy to break it into smaller chunks if that helps? My intent was only to make the document more semantic and easier to edit in future. If not, no worries. |
That's a good idea regardless. "This PR changes the headings to h1-h6" is something that's simple to review. Stacking it with unrelated changes makes it harder to review and harder to merge (like here due to concerns about indents and margins). |
No need to apologize edent, you couldn't have known. But I checked the changes and saw that you changed many more things like chapter title separation, emphasis dashes, title capitalization in addition to indents. All of these design choices are specifically made to improve the readability of this document. Prettiness comes after functionality in this guide. I am open to improvements but you should be able to explain how your changes improve the readability, based on design and usability principles. Semantic HTML idea looks useful and if it helps accessibility tools I am all for it. So I ask you to please make a PR that includes only "div to H element" as Frenzie said above. |
I hope this is OK. I've rewritten this to use semantic HTML. That makes it easier to navigate the document (especially if you use accessibility tools). I've made some light changes to the CSS.
If you're happy with these changes, I'll make the CSS and HTML more consistent.
I haven't changed the order of the text, but it's now easier to see that the layout is:
This change is