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

Region selector #556

Merged
merged 33 commits into from
Mar 17, 2023
Merged

Region selector #556

merged 33 commits into from
Mar 17, 2023

Conversation

seanchoi-dev
Copy link
Contributor

@seanchoi-dev seanchoi-dev commented Mar 13, 2023

  • Easily switch between locales while editing or viewing a page.
  • Re-organized how tools are displayed in Sidekick.
  • Opens Word, hlx.page, hlx.live in new window when clicked.

Resolves: MWPW-127468

Test URLs:
Word Referrer (edit mode)
Page Referrer (view mode)

@seanchoi-dev seanchoi-dev added the do not merge PR should not be merged yet label Mar 13, 2023
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Mar 13, 2023

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #556 (24270a0) into main (6c1ae57) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #556   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files          94       94           
  Lines       25092    25092           
=======================================
  Hits        24050    24050           
  Misses       1042     1042           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

item.append(previewLink);
item.append(editLink);
li.append(item);
ol.append(li);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicer if results are alphabetized.

Using something like:

const insertAlphabetically = (ol, li) => {
  const locale = li.getAttribute('data-locale');
  const items = [...ol.getElementsByTagName('li')];
  const insertBefore = items.find((item) => locale < item.getAttribute('data-locale'));
  if (insertBefore) {
    ol.insertBefore(li, insertBefore);
  } else {
    ol.append(li);
  }
};
Suggested change
ol.append(li);
insertAlphabetically(ol, li);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip. Applied!

Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com>
Copy link
Contributor

@rgclayton rgclayton left a comment

Choose a reason for hiding this comment

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

Looks good. Need to get Chris' PR merged into this.
There are a few code checks that are failing related to referrers

@rgclayton rgclayton self-requested a review March 17, 2023 16:59
@auniverseaway auniverseaway removed the do not merge PR should not be merged yet label Mar 17, 2023
@auniverseaway auniverseaway self-assigned this Mar 17, 2023
@auniverseaway auniverseaway added the verified PR has been E2E tested by a reviewer label Mar 17, 2023
@auniverseaway auniverseaway merged commit a71801d into main Mar 17, 2023
@auniverseaway auniverseaway deleted the region-selector branch March 17, 2023 23:36
chrischrischris added a commit to adobecom/milo-target that referenced this pull request May 22, 2023
* Adding tool button.

* WIP

* wip

* for testing.

* For testing

* Fixing few stuff after testing.

* Adjust palette size, change div to ol

* Sort the locale array alphabetically.

* nit: little refactoring.

* UI/UX update.

* remove white bg

* Added guide line for each item

* Fail condition added.

* Update libs/blocks/region-selector-loader/region-selector-loader.js

Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com>

* Print result alphabetically.

* nit: update getAttribute('data-locale') to dataset.locale

* Color code added for un-previewed webPath.

* Updated style as Rikkio and Ryan agreed.

* Taking off the test code.

* UI/UX update and enabling it on pages as well.

* Renaming the block

* Taking off the test urls

* Added search.

* Matching the search bar size as icons.

* Fixed the set logic.

* Polish (adobecom#575)

* Fix security issue

* Fix security issue

---------

Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com>
Co-authored-by: Chris Millar <cmillar@adobe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-impact Any PR that may affect consumers verified PR has been E2E tested by a reviewer
Projects
None yet
5 participants