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

Sync hazard statement presentation #3231

Merged

Conversation

Projects
4 participants
@infotexture
Copy link
Member

commented Feb 21, 2019

The default HTML5 output for hazard statements from #3207 currently differs from the PDF style in several ways:

  1. No padding in the table cells, so the text content nearly touches the borders
  2. Poor contrast with black text in the blue & red header rows
    (PDF uses white text in these cases)
  3. Header rows are left-aligned, whereas PDF centers them
  4. PDF uses all caps for all header rows, whereas HTML only does that for CAUTION & DANGER.
  5. PDF italicizes header text for notes & related variants
  6. Colors differ for the following types (blue in PDF, yellow in HTML):
    • fastpath
    • important
    • other
    • remember
    • restriction
    • tip
  • Sync header variants with PDF attribute sets
  • Sync Sass-generated CSS from HTML5 to XHTML

HTML state on develop vs. this PR branch

Before After
html-hazards-develop html-hazards-new

PDF

pdf-hazards

infotexture added some commits Feb 20, 2019

Add cell padding to hazard statement tables
Signed-off-by: Roger Sheen <roger@infotexture.net>
Use light text for hazards with dark backgrounds
Signed-off-by: Roger Sheen <roger@infotexture.net>
Center-align hazard statement header rows
Signed-off-by: Roger Sheen <roger@infotexture.net>
Uppercase all hazard statement header rows
Signed-off-by: Roger Sheen <roger@infotexture.net>
Italicize header text for notes & related variants
Signed-off-by: Roger Sheen <roger@infotexture.net>

@infotexture infotexture requested review from robander and jelovirt Feb 21, 2019

@infotexture

This comment has been minimized.

Copy link
Member Author

commented Feb 21, 2019

This PR addresses № 1–5 above. Regarding the colors in № 6, I assume the current PDF implementation is closer to the spec(s), but I wasn't certain.

I understand from @robander that #3207 re-uses capitalization code from the PDF implementation, but it seems better to delegate that to Sass/CSS here, to make it easier for users to override.

@jelovirt Any preferences?

@infotexture infotexture added this to In progress in 3.3 via automation Feb 21, 2019

@infotexture infotexture self-assigned this Feb 21, 2019

@infotexture infotexture moved this from In progress to Needs review in 3.3 Feb 21, 2019

@robander

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

I don't get quite the same default results from develop that you do, which makes me realize some of this is probably down to browser defaults -- for example all of the headers in my test from develop are centered (using Firefox). Pretty clear though that it's better to have that and similar defaults explicit in the CSS.

@robander

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

FWIW -- I have little knowledge of the specification here, I was generally just trying to give decent output that matched the PDF. Clearly I overlooked some differences (like the white text on blue background), missed some due to browser defaults (like centered text), and didn't even consider the extra padding around the text. Thanks for the close eye @infotexture when reviewing.

3.3 automation moved this from Needs review to Reviewer approved Feb 21, 2019

@infotexture

This comment has been minimized.

Copy link
Member Author

commented Feb 21, 2019

@robander & @jelovirt Thanks for the comments and approval, but the main question (№ 6) remains:

How should we handle the 6 hazard statement types which currently have different colored headers in PDF & HTML? I don't know which is "more correct", but I think users would at least expect us to treat each type consistently between output formats.

Once that's clear, we would also need to push another commit to sync the modified styles to XHTML output.

@robander

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

@infotexture I think unless somebody says it's wrong, I'd go with PDF and make them default to blue. Best to go with what we've already shipped there and keep them consistent.

@4everJang

This comment has been minimized.

Copy link

commented Feb 21, 2019

I purchased the ANSI Z535.6 standard years ago as I was working in machine industry. The hazard statement was meant to be compliant to this standard but got a couple of things wrong. ANSI only defines 4 levels of hazard statements, related to the effects of not avoiding these dangers: notice, caution, warning and danger. Any other type for hazard statement should not show a header at all, as they will have negative impact on the actual hazard statements. If a header is shown I would remove all coloring as well as the symbol from them - to take them away from the true hazard statement look and feel. I am attaching an image for the four ANSI level headers taken from the standard.
ansi-headers
The space left of the message panel is reserved for more specific hazard symbols, such as high voltage or sharp edges. In caution, warning or danger statements the standard symbol for that level can be repeated but it can also be a more specific version such as high voltage or sharp edges. In a NOTICE, this should NOT be a yellow triangle but can be a symbol like gas mask or gloves etc. (blue symbols indicating personal protective equipment). This area is something the OT does not need to handle as that is what the hazard symbol is supposed to do.

infotexture added some commits Feb 21, 2019

Sync hazard statement styling from HTML to XHTML
Amends 2b3f23e.

Signed-off-by: Roger Sheen <roger@infotexture.net>

3.3 automation moved this from Reviewer approved to Needs review Feb 21, 2019

3.3 automation moved this from Needs review to Reviewer approved Feb 21, 2019

@infotexture infotexture added this to the 3.3 milestone Feb 27, 2019

@infotexture infotexture merged commit 54ed9ec into dita-ot:develop Feb 27, 2019

3 checks passed

DCO DCO
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

3.3 automation moved this from Reviewer approved to Done Feb 27, 2019

@infotexture infotexture deleted the infotexture:feature/sync-hazard-presentation branch Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.