MWPW-142590 - [Share] enhancement - ability to edit text above icons #2070
Conversation
ryanmparrish
requested review from
rgclayton,
Sartxi,
So1ene,
Ruchika4,
elan-tbx and
vgoodric
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2070 +/- ##
==========================================
+ Coverage 96.35% 96.40% +0.04%
==========================================
Files 165 165
Lines 43534 43558 +24
==========================================
+ Hits 41947 41991 +44
+ Misses 1587 1567 -20 ☔ View full report in Codecov by Sentry. |
libs/blocks/share/share.js
Outdated
| const childDiv = rows[0].querySelector(':scope > div'); | ||
| const emptyRow = childDiv?.innerText.trim() === ''; | ||
| /* eslint-disable no-confusing-arrow,no-useless-escape */ | ||
| const toSentenceCase = (str) => str && typeof str === 'string' ? str.toLowerCase().replace(/(^\s*\w|[\.\!\?]\s*\w)/g, (c) => c.toUpperCase()) : ''; |
There was a problem hiding this comment.
nit: If we're already disabling eslint for this line, do you think we could break it out into multiple lines for better readability?
There was a problem hiding this comment.
Ah yep. Good call.
I added the /* eslint-disable-next-line no-useless-escape */ part
(I tested removing those 'useless' escapes and it didn't 'Sentence Case' right so I left that part as is)
I adjusted the arg into a few lines for better readability and fixes 'no-confusing-arrow' clause. :)
Thanks for the feedback
|
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. |
|
This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label. |
|
This PR is currently in the |
|
Hi @ryanmparrish ! Please do not forget to assign a QA engineer to verify the changes. |
|
@elan-tbx I assigned this to you for a verified label. Thanks |
elan-tbx
added
verified
|
Reminder to set the |
…dobecom#2070) * Check for block content and use as header if exists, updated twitter icon ref * Refactored to allow for authoring icon discression * share.test for new custom title, refactored so .inline doesn't have title support * remove test console * linting * eslint adjust better readability
Authors would like to modify the 'Share this page' text that appears above the social icons in the
Shareblock.XversionNote: The variant
.inlinedoes not support a title and strips out any authored text in the block content.Resolves: MWPW-142590
Test URLs:
Authoring Docs
Testing notes: To author a custom title in the
Shareblock, simply add the title in the blocks first content row. Works with or without the custom share url authoring options.