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

Add story containing inline link to Paragraph #746

Merged
merged 11 commits into from
Jul 4, 2019

Conversation

pjlee11
Copy link
Contributor

@pjlee11 pjlee11 commented Jul 1, 2019

Resolves #721

Overall change: Adds a story for the Paragraph to contain an Inline Link and pulls in @bbc/[email protected]

Code changes:


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

@pjlee11 pjlee11 added blocked This issue should not be worked on until another internal issue is completed - see desc for details ws-articles Tasks for the WS Articles Team labels Jul 1, 2019
@pjlee11 pjlee11 self-assigned this Jul 1, 2019
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jul 1, 2019

blocked on #716

@pjlee11 pjlee11 changed the title Add containing inline link story to Paragraph Add story containing inline link to Paragraph Jul 1, 2019
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jul 2, 2019

image

image

@pjlee11 pjlee11 removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 2, 2019
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jul 3, 2019

image

image

@sareh
Copy link
Contributor

sareh commented Jul 4, 2019

Discussing this with @jamesbrumpton & @nataliesmart:
When the text for the link is exactly the width of the viewport and you hover/focus, the styling adds padding of 2px on each side and therefore pushes a word down onto the next line. This jump is quite jarring. [1]
However, we discussed how the inline link item is to be used within an article body only - so in a paragraph in the main body copy, or a paragraph within a caption (for an image or video) and in all these scenarios, the components have a minimum of 8px spacing on either side of the text. So this 'jumping' behaviour will not be seen in an Article. [2]
However, we should add a note to the Readme of this component, under usage, to strongly suggest margin or padding on the left and right of 2px or more, whenever using this in production.

[1] Without spacing on left and right:
inline implementation

[2] With spacing of 2px on left and right:
inline implementation with 2px spacing left and right

@jamesbrumpton
Copy link
Contributor

LGTM

@pjlee11 pjlee11 mentioned this pull request Jul 4, 2019
1 task
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jul 4, 2019

@sareh great spot, I've opened #775. Going to merge this PR and we can discuss the README in 775

@sareh
Copy link
Contributor

sareh commented Jul 4, 2019

@pjlee11 It was all spotted by @jamesbrumpton! I just added the gifs 😄 Fixing forwards sounds good!

@pjlee11 pjlee11 merged commit c9f29e7 into latest Jul 4, 2019
@pjlee11 pjlee11 deleted the paragraph-inline-link-update branch July 4, 2019 11:23
@jroebu14 jroebu14 mentioned this pull request Jul 10, 2019
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "with inline-link" story to paragraph
5 participants