search

centrica-engineering / nucleus-docs

Documentation for the Nucleus Design System
https://nucleus.design
2 stars 10 forks source link

Convert docs images to work for dark mode #636

Closed RobTobias123 closed 3 months ago

RobTobias123 commented 5 months ago

Outcome

Content guidance images on all docs to use transparent webps that work well on both white and dark mode backgrounds. Replacing the extra white that currently appears in the areas the pink labels reside. Enabling a better experience once dark mode is released.

Scope

andij commented 5 months ago

Here's a guide on creating screenshots for the Docs if it helps: https://github.com/centrica-engineering/nucleus-docs/wiki/guide-screenshot

RobTobias123 commented 4 months ago

Proof of concept PR for review please: https://github.com/centrica-engineering/nucleus-docs/pull/664

RobTobias123 commented 4 months ago

Having tried 8 different techniques to create the optimal balance between image quality and file-size, the following process appears to have the best results:

  1. Create a Playground with the component within a ns-panel. Preview in a browser.
  2. Go to Inspector, set the witdh to 1280px.
  3. Mouseover the ns-panel in the code, right click and 'Capture Node Screenshot' to get just the panel and contents.
  4. Drag the downloaded file onto the Figma Docs images file.
  5. Set the Frame to 1080px and allow 10px top and bottom before a label.
  6. Set the Frame background to transparent.
  7. Apply the pink labels as required (relatable to key).
  8. Export as x1 PNG.
  9. Open PNG in Photoshop, Save As Webp 'Lossless' ensuring transparency is present so it will work on dark mode. Observe naming convention.
  10. Copy file to the Docs repo at Src/Assets/Components/ns-xyz/guidance.webp
  11. Check local
  12. Commit and PR.

Note that this method does not include optimising through TinyPNG or similar due to it appearing to over-compress the image and increase interpolation of pixels, resulting in a poorer quality image. Photoshop does a pretty god job of optimising the file anyway and the file-size, (if slightly larger than if Tiny'd) is still much smaller than a PNG or JPG whilst retaining a decent clarity and transparency.

This is the best overall balance for these guidance images, but may be adjusted on different image content such as if there are large gradients or photography.

It is the most accurate representation as screenshot is direct from coded version and uses only the node area for consistent white margins at a consistent size screenshot keeping everything in scale.

RobTobias123 commented 4 months ago

POC - PR with latest image created using the method above. https://github.com/centrica-engineering/nucleus-docs/pull/664

RobTobias123 commented 4 months ago

Epic PR created containing the following;

Form:

NSX:

RobTobias123 commented 4 months ago

Question about the use of the placeholder image we use for Video. As Docs is a public facing website, do we have permission to use this image? Could be subject to © copyright laws. Recommend that we replace with a British Gas image or ensure that it is considered 'in the public domain' /commons licenced to avoid the ricks - I mean risk.

RobTobias123 commented 4 months ago

Following conversation and initial review of these were looking Drew provided some good information on how we might improve with angled leaders etc. So here's what I've experimented with as a Figma component that allows reshaping/resizing without distortion of the label and includes 4 orientation points each with a pair of opposite curves. I've also included the brackets too. These can then be reused across any docs image. They now have 'lines' rather than shapes looking as lines so that they don't distort when modifying and the round end stays round etc. The colour and thickness of the line is the same as that around the existing label (2px #666666) which give a neutral colour than can be seen on both white and dark backgrounds. There's. small explanation on the component page.

As a proof of concept, I've applied the changes to 2 images - the ns-accordion and the ns-alert (which has a bottom oriented instance). hey can be seen in the relative PRs above. Let me know if you have any feedback...

Variants created: Image

Choose/switch type from list: Image

How it looks including simulated dark mode... Image Image

Image Image

RobTobias123 commented 4 months ago

I also tightened up the vertical spacing so less overlap onto the transparency which was making leader look overly long on light mode.

RobTobias123 commented 4 months ago

With consideration to this morning's feedback that this is an improvement but could be further enhanced by increasing clarity/contrast on dark background, I have increased the line width by 1px and used #FF00CC as a more salient colour the grey (also less likely blend in with the brand colours of the component). This has the effect of increasing the endpoint slightly too. This can be seen in the example below for point A vs point B (original as above).

An alternative is X which takes the same style as B but uses the darker Slate for the leader and instead relies on a 2px white highlight that helps contrast on the dark background and cuts through what it overlaps.

Please let me know if you have feedback as to which you find clearer...

Image

RobTobias123 commented 4 months ago

In terms of accessibility the pink leader of A conforms on white for graphical objects at 3.42:1 and on grey-light at 3.13:1. However, that may be problematic when over darker or more complex areas. My recommendation would be to use the style of X which uses Slate with a white highlight for optimal clarity.

RobTobias123 commented 4 months ago

New style variant is working well in both light and dark with sufficient contrast and clarity.

Image

RobTobias123 commented 4 months ago

All current PRs have been updated to use the new style (above).

RobTobias123 commented 3 months ago

All PRs in epic are now ready for review. Please find the respective links above in https://github.com/centrica-engineering/nucleus-docs/issues/636#issuecomment-2239304904

andij commented 3 months ago

Here are the comments from my review:

RobTobias123 commented 3 months ago

All approved PRs merged into the epic branch (except ns-header which has a question for the team)

andij commented 3 months ago

I've added all the review comments I summarised in my comment above to the relevant image in the pull request.

RobTobias123 commented 3 months ago

Updated and sequence upto and including ns-landmark (see PRs on #697 ) - to be continued

andij commented 3 months ago

I have reviewed the PRs that you've raised. Nice work!

I've added a comment to a couple of the PRs:

RobTobias123 commented 3 months ago

Updated #704 according to review comments.

RobTobias123 commented 3 months ago

Updated #703 according to review comments.

RobTobias123 commented 3 months ago

Thnaks for the further review and suggestions @andij - along with the updated image these have all now been committed and pushed to the PR for approval.

andij commented 3 months ago

@RobTobias123,

I've approved your recent changes 👍

The following PRs are available to be merged into the epic:

Note: Here's a filter that will list all PRs with your epic as their base: https://github.com/centrica-engineering/nucleus-docs/pulls?q=is%3Apr+is%3Aopen+base%3Aepic%2Fconverted-guidance-images

RobTobias123 commented 3 months ago

All PRs merged to Epic

andij commented 3 months ago

The last few outstanding comments to be resolved on the Epic PR:

RobTobias123 commented 3 months ago

All the above outstanding comments have been assessed, updated and marked as resolved. Please review and approve. PR #697

RobTobias123 commented 3 months ago

closing as completed