Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

New inline link styling #707

Merged
merged 23 commits into from
Jul 2, 2019
Merged

New inline link styling #707

merged 23 commits into from
Jul 2, 2019

Conversation

pjlee11
Copy link
Contributor

@pjlee11 pjlee11 commented Jun 25, 2019

Resolves #397
Resolves #716

Overall change: Adds the new UX inline link styling which is a background colour on hover. Also maintains the border changing from 1px to 2px on hover for users in high constrast modes.

Code changes:

  • On hover/focus the background becomes red and the text becomes white
  • On hover/focus adds pre-wrap so that for links that wrap to new lines the letter on the end of the line has a slight background buffer
  • Maintains a 2px border on hover/focus so there is a visual state change on high contrast modes
  • On hover/focus adds minus margin and padding to increase the background for buffer between the end of the background colour and the actual letter (see screenshots with and without in comments)

Testing notes:

This is most easily tested by following the instructions on #719 and then testing as described in the notes below. Happy to pair with test on this work.

  • Open storybook and regression test the new inline link styling using the caption and paragraph stories that contain inline links, make sure to check different world services with varying scripts and line-heights. NB: The reason we should not use the inline-link story for this regression is that it does not set it's own typography styling as it's expected to be used as a child of another component
  • Check firefox high contrast mode (Go to FireFox -> Preference -> Language & Appearance -> Colours -> 'Override the colours ...' set as Always)
  • Check Windows high contrast mode
  • Check with page zoomed to high percentages (300%+)
  • Check across different breakpoints
  • Check across different browsers
  • Check Inline link hover styling overlaps when the user is zoomed in #397 is fixed as part of this

  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@pjlee11 pjlee11 added ws-articles Tasks for the WS Articles Team articles-av-epic labels Jun 25, 2019
@pjlee11 pjlee11 self-assigned this Jun 25, 2019
@pjlee11 pjlee11 added the external blocked Blocked by an external dependency label Jun 25, 2019
@pjlee11 pjlee11 removed the external blocked Blocked by an external dependency label Jun 27, 2019
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jun 27, 2019

Without minus margin and padding

image

With minus margin and padding

image

Without minus margin & padding (Italic)

image

With minus margin & padding (italic)

image

Showing how the padding is affecting the element's background (Italic)

image

@pjlee11
Copy link
Contributor Author

pjlee11 commented Jun 27, 2019

Going to move this back to "PR in progress" and update the paragraph and caption stories to give a realistically storybook example of the inline link

@pjlee11
Copy link
Contributor Author

pjlee11 commented Jun 28, 2019

Firefox high contrast mode
firefox highcontrast mode

Copy link
Contributor

@RayNjeri RayNjeri left a comment

Choose a reason for hiding this comment

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

Tested it out in high contrast mode. Everything seems to be work as expected.
LGTM!

@pjlee11 pjlee11 merged commit 1ff3552 into latest Jul 2, 2019
@pjlee11 pjlee11 deleted the new-inline-link-styling branch July 2, 2019 14:07
@pjlee11 pjlee11 mentioned this pull request Jul 3, 2019
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
articles-av-epic ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New inline link styling Inline link hover styling overlaps when the user is zoomed in
4 participants