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-154795] Style Feds Global-footer region picker drop-up variant (without hash) #2599

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

sonawanesnehal3
Copy link
Contributor

@sonawanesnehal3 sonawanesnehal3 commented Jul 22, 2024

Add styling for region picker drop-up variant (without hash)

Resolves: MWPW-154795

Test URLs:

Before styles applied :
image

After styles applied :
image

QA : Sample Page

@sonawanesnehal3 sonawanesnehal3 added the needs-verification PR requires E2E testing by a reviewer label Jul 22, 2024
Copy link
Contributor

aem-code-sync bot commented Jul 22, 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 Jul 22, 2024

Page Scores Audits Google
/drafts/snehal/unav11 PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.86%. Comparing base (31c21e8) to head (01645e9).
Report is 10 commits behind head on stage.

Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #2599      +/-   ##
==========================================
- Coverage   95.86%   95.86%   -0.01%     
==========================================
  Files         176      176              
  Lines       46211    46211              
==========================================
- Hits        44300    44298       -2     
- Misses       1911     1913       +2     

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

@narcis-radu
Copy link
Contributor

@sonawanesnehal3 - I remember @overmyheadandbody implemented support for this already, I am able to see it running with forcing the global-footer component on blog - https://main--blog--adobecom.hlx.page/?footerqa=global-footer. Can you check whether your enhancement is needed or this can be addressed from authoring? Thanks!
Also, if a PR is needed, please provide test URLs in the before/ after area to make sure we capture the changes in the reports.

@sonawanesnehal3
Copy link
Contributor Author

Thanks @narcis-radu.
It was not added to Milo's styles. Blog is using custom styles.css to fix this issue.
Adding this fix to Milo, since all consumers who wants to use drop-up can leverage this .
Sure, I will provide test URLs in the before/ after area.

@spadmasa
Copy link

Hi @sonawanesnehal3 , can you please confirm on the region selection in locale page , when JP locale is updated in region doc, on change region selection is still seen as US.Can you please advise on this.
https://dropup--milo--adobecom.hlx.page/jp/drafts/snehal/unav11
image

@bandana147
Copy link
Contributor

bandana147 commented Jul 23, 2024

Hi @sonawanesnehal3 , can you please confirm on the region selection in locale page , when JP locale is updated in region doc, on change region selection is still seen as US.Can you please advise on this. https://dropup--milo--adobecom.hlx.page/jp/drafts/snehal/unav11 image

Please test the localisation flow in this PR as localisation flow is fixed in this PR.

@spadmasa
Copy link

spadmasa commented Jul 23, 2024

Validation done on the below url :
https://dropup--milo--adobecom.hlx.page/drafts/Sou/universalnav/unav3
image
image

image
image
loc flow will be verified as part of #2560

@spadmasa spadmasa added verified PR has been E2E tested by a reviewer Ready for Stage and removed needs-verification PR requires E2E testing by a reviewer labels Jul 23, 2024
@milo-pr-merge milo-pr-merge bot merged commit cebd090 into stage Jul 23, 2024
24 checks passed
@milo-pr-merge milo-pr-merge bot deleted the dropup branch July 23, 2024 14:21
@milo-pr-merge milo-pr-merge bot mentioned this pull request Jul 23, 2024
@overmyheadandbody
Copy link
Contributor

This may need to be reverted, as it affects current behavior:

Current look: https://main--milo--adobecom.hlx.page/drafts/ramuntea/footer-refactor-variant
Screenshot 2024-07-23 at 16 32 12

Look after merging this PR: https://stage--milo--adobecom.hlx.page/drafts/ramuntea/footer-refactor-variant
Screenshot 2024-07-23 at 16 32 19

I'm still not clear on what the intention for this PR was, as the logic and styles were working as expected.

CC: @bandana147, @sharmrj, @spadmasa, @narcis-radu

@overmyheadandbody
Copy link
Contributor

Also tried looking at the links provided in this PR, but the logic is trying to load https://stage--milo--adobecom.hlx.page/federal/footer/fragments/regions1.plain.html, which doesn't exist

rohitsahu pushed a commit to rohitsahu/milo that referenced this pull request Jul 26, 2024
* stage:
  MWPW-154998 [MEP][MILO] Manifests do not execute in the right order when there is a disabled manifest (adobecom#2632)
  mwpw-154965: Fetch federal stage content from hlx.page instead of stage.adobe.com (adobecom#2618)
  Correct error messages for duplicate files on the stage to main workflow (adobecom#2621)
  MWPW-153245 [merch][analytics] dispatch wcomp events, and let default lh (adobecom#2610)
  Revert "MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274" (adobecom#2627)
  MWPW-128600 Locale Tool: Langstore points to langstore/en (adobecom#2615)
  Fix for errors in dynamically loaded scripts in test cases (adobecom#2619)
  MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274 (adobecom#2593)
  Bootstrapper script for milo feds blocks (adobecom#2560)
  Revert "[MWPW-152968] mWeb - Passing ECID to Branch.io banner - Implementation" (adobecom#2612)
  MWPW-146743 Improve Article Header Performance (adobecom#2577)
  MWPW-153808: fix duplicate tax label (adobecom#2614)
  MWPW-154026: Long CTAs fall in the second line in merch card footer (adobecom#2565)
  Revert "[MWPW-154795] Style Feds Global-footer region picker drop-up variant (without hash)" (adobecom#2611)
  [AUTOMATED-PR] Update imslib.min.js dependency (adobecom#2605)
  [MWPW-154795] Style Feds Global-footer region picker drop-up variant (without hash) (adobecom#2599)
  MWPW-143053 [MEP] Request for New Personalization Tag - CC Paid (adobecom#2604)
  [MWPW-152674] [Gray Box] Desktop gnav not hidden when device view is open (adobecom#2597)
  MWPW-150566 - 🆕 Editorial-Card block (adobecom#2533)

# Conflicts:
#	libs/deps/merch-card.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for Stage verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants