[MWPW-168315] - Auto set close captions for videos (AdobeTv) #4288
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
libs/blocks/adobetv/adobetv.js
Outdated
|
|
||
| kr: 'kor', | ||
| }; | ||
| export const updateCaptionsParam = (urlStr, geo) => { |
There was a problem hiding this comment.
We usually have a break between methods
Suggested change
| export const updateCaptionsParam = (urlStr, geo) => { | |
| export const updateCaptionsParam = (urlStr, geo) => { |
libs/blocks/adobetv/adobetv.js
Outdated
Comment on lines
109
to
110
| const localePrefix = config?.locale?.prefix || ''; | ||
| const geo = localePrefix.replace('/', '') ?? ''; |
There was a problem hiding this comment.
The ?? '' part is redundant, since you're already using an empty string as a fallback for the localePrefix. So you could do:
Suggested change
| const localePrefix = config?.locale?.prefix || ''; | |
| const geo = localePrefix.replace('/', '') ?? ''; | |
| const geo = (config?.locale?.prefix || '').replace('/', ''); |
libs/blocks/adobetv/adobetv.js
Outdated
| const config = getConfig(); | ||
| const localePrefix = config?.locale?.prefix || ''; | ||
| const geo = localePrefix.replace('/', '') ?? ''; | ||
| const captionHref = updateCaptionsParam(a.href, geo); |
There was a problem hiding this comment.
The naming is misleading. This is not a separate captions href, it's the actual href of the video. Something like videoHref or something similar would make its intention clearer.
libs/blocks/adobetv/adobetv.js
Outdated
Comment on lines
99
to
101
| const newCaption = CaptionMap[geo]; | ||
| if (newCaption) { | ||
| url.searchParams.set('captions', newCaption); |
There was a problem hiding this comment.
I also see a bit of a naming issue here, you're updating the captions language, which isn't clearly portrayed. newCaptionLang or something similar might be better suited.
libs/blocks/adobetv/adobetv.js
Outdated
Comment on lines
5
to
93
| ae_ar: 'eng', | ||
| ae_en: 'eng', | ||
| africa: 'eng', | ||
| au: 'eng', | ||
| be_en: 'eng', | ||
| bg: 'eng', | ||
| ca: 'eng', | ||
| cz: 'eng', | ||
| dk: 'eng', | ||
| ee: 'eng', | ||
| gr_en: 'eng', | ||
| hk_en: 'eng', | ||
| hu: 'eng', | ||
| id_en: 'eng', | ||
| id_id: 'eng', | ||
| ie: 'eng', | ||
| il_en: 'eng', | ||
| il_he: 'eng', | ||
| in: 'eng', | ||
| lt: 'eng', | ||
| lu_en: 'eng', | ||
| lv: 'eng', | ||
| mena_ar: 'eng', | ||
| mena_en: 'eng', | ||
| my_en: 'eng', | ||
| my_ms: 'eng', | ||
| no: 'eng', | ||
| nz: 'eng', | ||
| ph_en: 'eng', | ||
| ph_fil: 'eng', | ||
| pl: 'eng', | ||
| ro: 'eng', | ||
| ru: 'eng', | ||
| sa_ar: 'eng', | ||
| sa_en: 'eng', | ||
| sg: 'eng', | ||
| si: 'eng', | ||
| sk: 'eng', | ||
| th_en: 'eng', | ||
| tr: 'eng', | ||
| ua: 'eng', | ||
| uk: 'eng', | ||
| vn_en: 'eng', | ||
| vn_vi: 'eng', | ||
| fi: 'eng', | ||
|
|
||
| be_fr: 'fre_fr', | ||
| ch_fr: 'fre_fr', | ||
| fr: 'fre_fr', | ||
| lu_fr: 'fre_fr', | ||
| ca_fr: 'fre_fr', | ||
|
|
||
| at: 'ger', | ||
| ch_de: 'ger', | ||
| lu_de: 'ger', | ||
| de: 'ger', | ||
|
|
||
| jp: 'jpn', | ||
|
|
||
| it: 'ita', | ||
| ch_it: 'ita', | ||
|
|
||
| es: 'spa', | ||
|
|
||
| br: 'por_br', | ||
| pt: 'por_br', | ||
|
|
||
| th_th: 'tha', | ||
|
|
||
| ar: 'spa_la', | ||
| cl: 'spa_la', | ||
| co: 'spa_la', 8000 | ||
| la: 'spa_la', | ||
| mx: 'spa_la', | ||
| pe: 'spa_la', | ||
|
|
||
| nl: 'dut', | ||
| be_nl: 'dut', | ||
|
|
||
| se: 'swe', | ||
|
|
||
| cn: 'chi_hans', | ||
|
|
||
| hk: 'chi_hant', | ||
| tw: 'chi_hant', | ||
|
|
||
| in_hi: 'hin', | ||
|
|
||
| kr: 'kor', |
There was a problem hiding this comment.
This feel extremely repetitive. How about using the languages as a key, with an array value containing the current relevant keys. For example, ger: ['at', 'ch_de', 'lu_de', 'de'],. You'd of course need to update the updateCaptionsParam method as well to search through the entries, but this alternative structure would keep things better organized, shorter and easier to update in the future.
There was a problem hiding this comment.
initially I thought about it but for a easier ans faster retrieval went ahead with the above approach. will make the changes.
|
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. |
raga-adbe-gh
approved these changes
Jun 4, 2025
There was a problem hiding this comment.
Thanks for making the updates! I have one more comment and I also noticed that the PSI checks are failing. For a PR to get merged, all the checks need to pass, so you'll either have to rerun the checks or re-author the page in such a way that PSI tests succeed. Same goes for the failing Nala tests - you can rebase your branch with the latest stage one to see if addresses the issue. If not, it might be that some integration tests need to be updated.
libs/blocks/adobetv/adobetv.js
Outdated
Comment on lines
28
to
44
| /** | ||
| * Maps a video URL's captions parameter to the appropriate language code based on geo location. | ||
| * | ||
| * @param {string} videoUrl - The URL of the Adobe TV video | ||
| * @param {string} geo - The geographic location code (e.g. 'fr', 'jp', 'de') | ||
| * @returns {string} The updated video URL with the correct captions language parameter | ||
| * | ||
| * @example | ||
| * // Updates captions to French for French geo | ||
| * updateCaptionsLang('https://video.tv.adobe.com/v/123456?captions=eng', 'fr') | ||
| * // Returns: 'https://video.tv.adobe.com/v/123456?captions=fre_fr' | ||
| * | ||
| * @example | ||
| * // Preserves URL if captions param not present | ||
| * updateCaptionsLang('https://video.tv.adobe.com/v/123456', 'fr') | ||
| * // Returns: 'https://video.tv.adobe.com/v/123456' | ||
| */ |
There was a problem hiding this comment.
Because there's no build to remove comments/minimize code/etc., we try to avoid JSDoc comments, since these add weight to the end file, without adding customer value. This method is straightforward enough to not need all the documentation.
Suggested change
| /** | |
| * Maps a video URL's captions parameter to the appropriate language code based on geo location. | |
| * | |
| * @param {string} videoUrl - The URL of the Adobe TV video | |
| * @param {string} geo - The geographic location code (e.g. 'fr', 'jp', 'de') | |
| * @returns {string} The updated video URL with the correct captions language parameter | |
| * | |
| * @example | |
| * // Updates captions to French for French geo | |
| * updateCaptionsLang('https://video.tv.adobe.com/v/123456?captions=eng', 'fr') | |
| * // Returns: 'https://video.tv.adobe.com/v/123456?captions=fre_fr' | |
| * | |
| * @example | |
| * // Preserves URL if captions param not present | |
| * updateCaptionsLang('https://video.tv.adobe.com/v/123456', 'fr') | |
| * // Returns: 'https://video.tv.adobe.com/v/123456' | |
| */ |
* [MWPW-173016] - block korea free trial links/buttons * [MWPW-173016] - korea text filter added * [MWPW-173016] - update if * [MWPW-173016] - code optimization * [MWPW-173016] - improve import * [MWPW-173016] - modal check, string check added * [MWPW-173016] - strings added * [MWPW-173470] - update logic * [MWPW-173470] - update logic * [MWPW-173470] - update logic * [MWPW-173470] - remove whitespace * [MWPW-173470] - optimize * [MWPW-173470] - remove variable * [MWPW-173016] - optimize code * [MWPW-173016] - group logic * [MWPW-173016] - merch solution added * [MWPW-173016] - merch ref comment added * [MWPW-173470] - null safe * [MWPW-173470] - code optimization * [MWPW-173470] - nav korea restrict coverage * [MWPW-173470] - fix eslint console error --------- Co-authored-by: Dusan Kosanovic <dusan.kosanovic@hitthecode.com>
…ssue (adobecom#4177) * fixed visual list is not marked up as list a11y issue * fixed eslint * Fixed footer alignment * refactored the code * fixed review comments
* MWPW-169219: Add tooltip to aside promo popup * MWPW-169219: Move tooltip icon to the right on mobile * MWPW-169219: Move tooltip icon on mobile to the left * MWPW-169219: Set tooltip font size to heading font size and align it * MWPW-169219: Add support for rtl * MWPW-169219: Pr update * MWPW-169219: PR feedback * MWPW-169219: Set tooltip height to 16px
… has not loaded (Desktop & Mobile) (adobecom#4213) * Parallelize the network calls required before the gnav becomes visible * corrected the logic for loading base styles for non-milo contexts * load aside.js in parallel if necessary * made the logic inside getGnavSource a little more fault tolerant * Fixed an issue with loading the dark mode gnav (standalone) * changed the gnav tests to reload the global-navigation.js module anew on each test to work correctly with the performance optimizations * Fixed a test for localnav inside bootstrapper.test.js * ignore eslint error on a test * added error handling to the style loading logic in gnav * changed error handling for loading gnav styles from throwing an error to rejecting a promise * removed some redundant code * changed e.message to e * implemented the loading state for desktop viewports * Change animation name as it was the same as one that the unav uses * implemented the loading state for mobile * Removed pointer events from the loading state; made the loading state closer to the specs * Finishing touches * Don't have a loading state for the legacy mobile gnav * add the loading state only on async Dropdowns; decorateLocalnav should be inside decorateMenu.then(...) * put the call to `decorateLocalNavItems` inside `finally` to make sure it runs * lint fixes * Sometimes mobileNavCleanup can be null * Removed an extraneous line * Cleaning up the merge conflict resolution * Make sure `transformTemplateToMobile` always returns a function * Revert "Revert "MWPW-167709 [GNAV] [Pre-Load] | (Ghost) State when Mega Menu fragment has not loaded (Desktop & Mobile)" (adobecom#4210)" This reverts commit 430f850. * Fixed and issue where the gnav popup was emoty on the homepage when switching between mobile and desktop viewports
* Fix for jarvis A11Y issue with tooltip * lint fix --------- Co-authored-by: Dev Ashish Sardana <glo77801@adobe.com>
* [MWPW-171938] - cell role fix * [MWPW-171938] - aria hidden added for empty cells * [MWPW-171938] - revert aria hidden * [MWPW-171938] - shorten code
* [MWPW-173963] - table inside tabs sticky fix * [MWPW-173963] - update logic * [MWPW-173963] - code optimization, variable name change
…om#4252) * fixing advertising and eventmergeid * fixing error * fixing error * fixing error * fixing error * fixing error * fixing error * Adding BACOM properties * Updating DS IDs for BACOM pages * Adding aem * fixing errors * fixing errors * fixing errors * fixing errors * fixing errors * fixing errors --------- Co-authored-by: Akansha Arora <>
play aria fix Co-authored-by: Drashti Modasara <dmodasara@Drashtis-MacBook-Pro.local>
…g feature (adobecom#3963) fixed with css
…dobecom#4061) * Gap is seen between promo banner and G-nav when page is scrolled. * move the changes to global navigation css * add min height after 1 second * instead of 100% use 72px as an actually banner height * use --global-height-navPromo instead of static 72px
* WP * WP * wp * Fix a few small issues * Set max length for finishing logs * Remove unused env var * Fix linting issues * add org * Add PR feedback * Add yaml to trigger bacom imports * Always log the first 500 successfully imported live paths * Add EOF (thx Rares) * Dont error if there are no logs * Remove double slack notification * Add Rares feedback
…m#4286) * Set the correct path from which the workflow is running from * Allow to set a local LAST_ISO_RUN for better debugging
* fix fallback deeplink * fix edu * fix * fix cs * fix tests * fix nala&unit tests * fix analytics * fix * fix flacky test * fix another flacky test * add fallback nala test * add test for iframe src
* Improve carousel accessibility * add visibility hidden * clear timeout * update nala tests * set aria-hidden for non visible slides * PR feedback * PR feedback * Update css comments
…e them treated as text. (adobecom#4276) * Update table.js * Update table.js * Update table.js * Update table.js
* Add personalization roc * Remove forced order and simplify tests * Remove unused json * Update createManifests func name * [MMM][MEP] "MMM data for last 7 days" should not appear in the MMM report (adobecom#4230) * Initial checkin. Good state. * Naming conventions. Fix sticky marquee logic. * Variant name update. * Alignment fix. * Update with hide at action area logic. * Updated as per PR comment. * Marquee updates, first pass. * Var1 marquee button rework. * Possible custom-hide fix. * Initial checkin. Fetch manifests API. Generate HTML. * Headers, divider logic, change events, hide selected, 'non' select option logic. * Update to duplicate manifest logic. * Remove console log. * Linting. * Unit test update. * Label/select rename. Don't update button for hidden experiences. Default MMM to none. * Hidden option button update fix. * Unit Testing. * Update libs/features/personalization/preview.js * Remove placeholder domain. * Update manifest option titles for clarity in preview.js * Update option title for clarity in getManifestListDomAndParameter function * Initial checkin. Remove 'more manifests' checkbox from MMM. --------- Co-authored-by: Vivian A Goodrich <101133187+vgoodric@users.noreply.github.com> Co-authored-by: Vivian A Goodrich <vgoodric@adobe.com> * MWPW-174288 [MMM][MEP] Fix stying and DOM issue in new MMM report (adobecom#4260) [MMM]: Added styling fix for MMM variants Co-authored-by: Denys Fedotov <dfedotov@Denyss-MacBook-Pro.local> Co-authored-by: Vivian A Goodrich <vgoodric@adobe.com> --------- Co-authored-by: Vivian A Goodrich <vgoodric@adobe.com> Co-authored-by: Dave Linhart <132396886+AdobeLinhart@users.noreply.github.com> Co-authored-by: Vivian A Goodrich <101133187+vgoodric@users.noreply.github.com> Co-authored-by: Denys Fedotov <denlight@gmail.com> Co-authored-by: Denys Fedotov <dfedotov@Denyss-MacBook-Pro.local>
…mily to localnav (adobecom#4211) * Making new nav true by default for standalone gnav * Fixing test case
…e and it overlaps (adobecom#4223) * Making aside z index 0 if appswitcher is open * Making z index to 11 if it switches back to desktop * Checking promo exist before calling updatePromoZIndex * Lint fix
[MWPW-170419] - width added to image container
* MWPW-173864: Updates milo Caas Configurator labels * Updates label
* MWPW-173197 Allow loading of gnav on all graybox domains With graybox, there will be many different domains used - e.g: https://04-02-25-nab-gen-studio-mweb.graybox.adobe.com When domains without https:// are added to `cdnWhitelistedOrigins`, then an `endsWith` check is done to see if the domain is allowed. * Add test case * Small Perf Optimization
When loading a milo page into an iframe, sometimes fragments would fail to load and would instead display as text. The reason for this is that `createTag` was using `instanceof HTMLElement`. However the instanceof Element check fails because the Element constructor we're checking against belongs to a different JavaScript realm (outside iframe) than the one where the element was created. Using myEl.nodeType === Node.ELEMENT_NODE is a more reliable, cross-context way to determine if an object is an HTML element.
…ecom#4243) url parse error fix
…#4201) * update the fleaky tests and aem.live url * all-elements * nested personalization * update to run.sh * skipping few more tests on webkit --------- Co-authored-by: Santoshkumar Sharanappa Nateekar <nateekar@Santoshkumars-MacBook-Pro.local> Co-authored-by: Santoshkumar Sharanappa Nateekar <nateekar@MacBookPro.attlocal.net>
) Revert "[MWPW-173717] Hyphenate large headings on mobile (adobecom#4199)" This reverts commit 7b82821.
Update utilities.js Making a change to the utilities file
|
closing this for now as the last git push had some issues. will raise a new one |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: MWPW-168315
Test URLs:
GNav Test URLs
Gnav + Footer + Region Picker modal:
Thin Gnav + ThinFooter + Region Picker dropup:
Localnav + Promo:
Sticky Branch Banner:
Inline Branch Banner:
Blog
RTL Locale