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

Page Attachment as linked content #2848

Merged
merged 76 commits into from Aug 24, 2020
Merged

Page Attachment as linked content #2848

merged 76 commits into from Aug 24, 2020

Conversation

miina
Copy link
Contributor

@miina miina commented Jun 30, 2020

Summary

Adds the Page Attachment features as linked content.

Relevant Technical Choices

To-do

  • Needs UX for the general behavior in case a link ends up in an invalid position.

  • UX adjustments for the editor

  • Add "default" suffix to Learn more

  • Behavior when there is a linked element

  • Refactor code a bit

  • Disable links for output that are in the Page Attachment area.

  • Tests (Need update post-UX change)

User-facing changes

There is a new Page Attachment panel which the user can add while Page/background is selected.
The moment when the user adds a URL, the Page Attachment gets added.

Testing Instructions

I

  1. Open a Page
  2. Add a URL to the Page Attachment
  3. Verify that if you add incorrect URL format (e.g. with a space inside), a warning is displayed

II

  1. Open a Page
  2. Add a URL to the Page Attachment
  3. Verify that "Learn more" appears in the bottom of the page
  4. Change the "Learn more" to something else.
  5. Verify that it also changed on the Page.
  6. Verify that removing the CTA Text will restore the default "Learn more"
  7. Verify that the Page Attachment is also displayed in the Front-end and with the correct CTA Text.

III

  1. Open a Page
  2. Add a URL to the Page Attachment
  3. Add a Shape, add a link to the Shape
  4. Try dragging/resizing and rotating the link so that part of the element is near the bottom -- where the "Learn more" is displayed.
  5. Verify that after finishing with the transformation, a dashed line appears with a tooltip on the left saying that links can't be below the dashed line with Page Attachment.

IV

  1. Open a Page
  2. Add a URL to the Page Attachment
  3. Add 3 elements, add link to one of these elements
  4. Select all 3 elements
  5. Drag/resize/rotate the elements so that the linked element is positioned near the "Learn more" text.
  6. Verify again the same dashed line with the tooltip.

V

  1. Add a shape with link
  2. Move the shape to the bottom area of the page
  3. Now click on the Page and click on the Page Attachment input without typing anything.
  4. Verify that the dashed line is displayed again and a warning message appears explaining that links of the elements below the dashed line won't be clickable in case of adding Page Attachment.
  5. Add a Page Attachment URL, save the story and verify that the link is not clickable indeed.

VI

  1. Add a Page Attachment
  2. Create a shape and move it to the Page Attachment area.
  3. Focus on the link field of the Shape element.
  4. Verify that a warning appears when focusing on the link input and the dashed line is displayed.
  5. Verify that it's not possible to fill in the link field.

Fixes #254

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #2848 into main will increase coverage by 0.10%.
The diff coverage is 92.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2848      +/-   ##
==========================================
+ Coverage   82.78%   82.88%   +0.10%     
==========================================
  Files         792      797       +5     
  Lines       13915    14055     +140     
==========================================
+ Hits        11520    11650     +130     
- Misses       2395     2405      +10     
Flag Coverage Δ
#karmatests 70.49% <88.65%> (+0.25%) ⬆️
#unittests 65.63% <48.22%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...s/src/edit-story/components/canvas/displayLayer.js 70.83% <ø> (ø)
...dit-story/components/panels/alignment/alignment.js 73.91% <ø> (ø)
...ets/src/edit-story/components/panels/link/index.js 80.39% <80.00%> (-0.38%) ⬇️
...ssets/src/edit-story/utils/useElementsWithLinks.js 88.46% <88.46%> (ø)
assets/src/edit-story/utils/isElementBelowLimit.js 88.88% <88.88%> (ø)
...it-story/components/canvas/pageAttachment/index.js 89.47% <89.47%> (ø)
...it-story/components/panels/pageAttachment/index.js 92.85% <92.85%> (ø)
...src/edit-story/components/canvas/canvasProvider.js 97.72% <100.00%> (+0.10%) ⬆️
assets/src/edit-story/components/form/link.js 100.00% <100.00%> (ø)
assets/src/edit-story/components/form/text.js 100.00% <100.00%> (ø)
... and 17 more

@github-actions
Copy link
Contributor

github-actions bot commented Jun 30, 2020

Size Change: +2.9 kB (0%)

Total Size: 1.15 MB

Filename Size Change
assets/js/edit-story.js 485 kB +1.95 kB (0%)
assets/js/stories-dashboard.js 510 kB +941 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 268 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/chunk-web-stories-template-0-********************.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-1-********************.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-2-********************.js 10 kB 0 B
assets/js/chunk-web-stories-template-3-********************.js 10.5 kB 0 B
assets/js/chunk-web-stories-template-4-********************.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-5-********************.js 6.93 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 9.91 kB 0 B
assets/js/chunk-web-stories-template-7-********************.js 9.78 kB 0 B
assets/js/web-stories-activation-notice.js 63.6 kB 0 B
assets/js/web-stories-embed-block.js 16.9 kB 0 B

compressed-size-action

@miina miina changed the title [WIP] Page Attachment as linked content Page Attachment as linked content Jul 7, 2020
@miina miina marked this pull request as ready for review July 7, 2020 16:34
@miina miina marked this pull request as ready for review August 17, 2020 14:13
@miina miina requested review from barklund and merapi August 17, 2020 14:14
Copy link
Contributor

@barklund barklund left a comment

Choose a reason for hiding this comment

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

Just a single comment, but otherwise looks great. Let's get input from one more before moving to QA though, as this is quite a big feature.

assets/src/edit-story/output/page.js Outdated Show resolved Hide resolved
@swissspidy swissspidy added Group: Page Attachment Type: Enhancement New feature or improvement of an existing feature labels Aug 18, 2020
@miina
Copy link
Contributor Author

miina commented Aug 20, 2020

@barklund @swissspidy If you have time, could you PTAL at this: 06031fd

Since the real update to the CTA text happened previously only onBlur then this didn't allow saving before onBlur had happened. This now uses a combination of locally kept _ctaText and debouncedCallback for allowing typing without lag + update the global ctaText in the background. Will send back to QA if it gets a 👍 from either of you.

Copy link
Contributor

@barklund barklund left a comment

Choose a reason for hiding this comment

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

I just have a single suggestion, otherwise it looks good.

@miina miina merged commit f56b5d2 into main Aug 24, 2020
@miina miina deleted the add/254-page-attachment branch August 24, 2020 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Page Attachment Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Page Attachments - linked content
7 participants