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

add styles for userguide and changes #12027

Merged
merged 6 commits into from Feb 3, 2021
Merged

add styles for userguide and changes #12027

merged 6 commits into from Feb 3, 2021

Conversation

feerrenrut
Copy link
Contributor

Link to issue number:

None

Summary of the issue:

The user guide has long since been missing any basic styling. While very utilitarian, the default appearance will likely be perceived by many as ugly and potentially hard to read.

Description of how this pull request fixes the issue:

Add simple style rules for the user guide, changes file, and key commands

Testing performed:

Tested opening these from NVDA.

Known issues with pull request:

None

Change log entry:

Section: Changes

- The user guide, changes file, and key commands listing now have a refreshed appearance.

@feerrenrut
Copy link
Contributor Author

Note: I haven't updated the appveyor file to package the css with the docs. For reviewing a build I don't think this is necessary. @michaelDCurran could you comment on what we would need to do to get the css file to accompanying the userguide / changes file to the website?

@Qchristensen, here is a demo:
styledUserGuide.zip

@michaelDCurran
Copy link
Member

Although styles.css was being copied to each language directory, it also needs to be copied to the output directory. Thus I just pushed a commit that ensures that styles.css is copied to the output directory if the userGuide or changes targets are requested to be build with scons.
As for getting styles.css included when we actually publish a snapshot or release: this will be a two-line change to our appveyorHook internally. But can't be done until alpha/beta snapshots start producing these with this pr.
Either this pr must be held back until NVDA 2020.4 is fully released, or this should go into beta.

@feerrenrut
Copy link
Contributor Author

I'm not sure I follow, can we merge this and just delay updating the appveyorHook until after 2020.4 is released?

@michaelDCurran
Copy link
Member

michaelDCurran commented Feb 3, 2021 via email

@feerrenrut
Copy link
Contributor Author

Ok, in that case I'll wait for @Qchristensen to review / approve the style choices and we can proceed.

Qchristensen
Qchristensen previously approved these changes Feb 3, 2021
Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. I agree, no urgency with merging it.

@Qchristensen
Copy link
Member

I didn't comment on the style, but I like the use of the purple. My only thoughts were

  • For tables if we could shade every second row might help reading visually.
  • Is it worth purring a HR or something to more clearly denote the change between topics?

I thought maybe changes like that which weren't critical could be done later down the track.

@feerrenrut
Copy link
Contributor Author

Thanks for taking a look @Qchristensen, yes those changes could be made fairly easily after this is merged. The other thing I was hoping we could do was to delineate shortcuts clearly.

I'll merge this in now. It should be visible when docs are launched from NVDA, but won't become available on our website until 2021.1 reaches beta.

@feerrenrut feerrenrut merged commit 1c0ab7c into master Feb 3, 2021
@feerrenrut feerrenrut deleted the addStylesUserDocs branch February 3, 2021 08:41
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants