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

MWPW-142590 - [Share] enhancement (resubmit) - ability to edit text above icons #2240

Merged
merged 8 commits into from
May 8, 2024

Conversation

ryanmparrish
Copy link
Contributor

@ryanmparrish ryanmparrish commented May 1, 2024

Resubmitting this PR

This was previously reverted due to an issue found on a live page. This was caused when a share block is authored w/ out a content row. See LIVE CC example below. (no authoring changes required)


Authors would like to modify the 'Share this page' text that appears above the social icons in the Share block.

Enabled author-able share titles
Updated the twitter logo use new X version

image
Note: The variant .inline does not support a title and strips out any authored text in the block content.

Resolves: MWPW-142590

Test URLs:

LIVE CC page: https://main--cc--adobecom.hlx.page/creativecloud/animation/discover/principles-of-animation?milolibs=rparrish-share-update

Before: https://main--milo--adobecom.hlx.page/drafts/rparrish/share?martech=off
After: https://rparrish-share-update--milo--adobecom.hlx.page/drafts/rparrish/share?martech=off
Authoring Docs

Before: https://main--milo--adobecom.hlx.page/docs/authoring/examples/share-block?martech=off
After: https://rparrish-share-update--milo--adobecom.hlx.page/docs/authoring/examples/share-block?martech=off

Testing notes: To author a custom title in the Share block, simply add the title in the blocks first content row. Works with or without the custom share url authoring options.

image

@ryanmparrish ryanmparrish added the needs-verification PR requires E2E testing by a reviewer label May 1, 2024
Copy link
Contributor

aem-code-sync bot commented May 1, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented May 1, 2024

Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 96.17%. Comparing base (88a24e8) to head (ffe6025).
Report is 42 commits behind head on stage.

Files Patch % Lines
libs/blocks/share/share.js 92.85% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #2240      +/-   ##
==========================================
- Coverage   96.66%   96.17%   -0.49%     
==========================================
  Files         165      165              
  Lines       43525    43615      +90     
==========================================
- Hits        42072    41948     -124     
- Misses       1453     1667     +214     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@skumar09 skumar09 added the run-nala Run Nala Test Automation against PR label May 1, 2024
Copy link
Contributor

github-actions bot commented May 2, 2024

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

@SilviuLCF
Copy link

SilviuLCF commented May 7, 2024

Verified : testing details
https://jira.corp.adobe.com/browse/MWPW-142590

Not adding label ready for stage yet since
@ryanmparrish there are still two checks on the PR that are not passing

CC @narcis-radu

@SilviuLCF SilviuLCF added verified PR has been E2E tested by a reviewer and removed needs-verification PR requires E2E testing by a reviewer labels May 7, 2024
@mokimo
Copy link
Contributor

mokimo commented May 8, 2024

Approving and merging to stage as this was already on prod #2070 and the issues have been fixed and verified

@mokimo mokimo merged commit b71f8a3 into stage May 8, 2024
19 of 22 checks passed
@mokimo mokimo deleted the rparrish/share-update branch May 8, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for Stage run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants