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

Update Image to include AmpImg & add srcset prop #241

Merged
merged 35 commits into from
Jan 10, 2019
Merged

Conversation

sareh
Copy link
Contributor

@sareh sareh commented Dec 20, 2018

Part of bbc/simorgh#929

  • Add AmpImg, update Image to include srcset prop

Testing notes
When you visit storybook, you should see two sets of stories in the menu. Image - AmpImg and Image - Img.
The AmpImg stories should load correctly, with all associated Amp HTML.

http://localhost:8080/?selectedKind=Image%20-%20AmpImg

screen shot 2018-12-31 at 13 08 47

screen shot 2018-12-31 at 13 08 35

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

@sareh sareh self-assigned this Dec 20, 2018
@sareh sareh mentioned this pull request Dec 31, 2018
4 tasks
@sareh
Copy link
Contributor Author

sareh commented Dec 31, 2018

Bug found: When running storybook npm install && npm run storybook and visiting an Amp Img story, http://localhost:8080/?selectedKind=Image%20-%20AmpImg&selectedStory=16%3A9%20landscape%20image it loads, but clicking through to a second or a thirds Amp Img story fails to render the <amp-img> image. When the page is refreshed, or if the link is visited directly, e.g. http://localhost:8080/?selectedKind=Image%20-%20AmpImg&selectedStory=image%20with%20srcset, the image does render.

I'd suggest making this a separate issue to this PR.

Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

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

Good start, thanks Sareh

packages/components/psammead-image/README.md Outdated Show resolved Hide resolved
packages/components/psammead-image/README.md Outdated Show resolved Hide resolved
packages/components/psammead-image/README.md Outdated Show resolved Hide resolved
packages/components/psammead-image/README.md Outdated Show resolved Hide resolved
packages/components/psammead-image/README.md Outdated Show resolved Hide resolved
packages/components/psammead-image/CHANGELOG.md Outdated Show resolved Hide resolved
@dr3 dr3 self-assigned this Jan 4, 2019
.storybook/config.js Outdated Show resolved Hide resolved
@sareh sareh dismissed stale reviews from ChrisBAshton, bcmn, pjlee11, and dr3 via ab5d1cf January 9, 2019 15:41
@jamesbrumpton
Copy link
Contributor

LGTM 👍

@dr3 dr3 merged commit 12c132e into latest Jan 10, 2019
@dr3 dr3 deleted the update-image-ampimg branch January 10, 2019 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants