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

feat: support cross origin request to tools and labs #236

Merged
merged 4 commits into from
Aug 28, 2024
Merged

Conversation

dylandepass
Copy link
Member

@dylandepass dylandepass commented Aug 26, 2024

Fixes: #235 and #237

@@ -22,6 +22,7 @@ export const style = css`
pointer-events: none;
z-index: 999999999999;
font-weight: initial;
letter-spacing: initial;
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed that sites modifying letter-spacing also had an impact on the sidekick.

@@ -62,7 +62,7 @@ export class Picker extends SPPicker {
} else if (!this.pointerdownState) {
// Prevent browser driven closure while opening the Picker
// and the expected event series has not completed.
this.overlayElement.manuallyKeepOpen();
this.overlayElement?.manuallyKeepOpen();
Copy link
Member Author

Choose a reason for hiding this comment

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

This starting causing some issues with one of the tests

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b8b4fc4) to head (f83efbe).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #236   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           52        52           
  Lines         1947      1957   +10     
=========================================
+ Hits          1947      1957   +10     

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

responseHeaders: [{
header: 'Access-Control-Allow-Origin',
operation: 'set',
value: '*',
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be set to tools.aem.live or labs.aem.live respectively?

Copy link
Member Author

Choose a reason for hiding this comment

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

How would I know which one to set though? This is set at the time of login (on their aem.page/live or host).

Think I should create two rules then?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think also initiatorDomains will have the same effect as being explicit with the ACAO header.

@dylandepass dylandepass merged commit 314e2b8 into main Aug 28, 2024
6 checks passed
@dylandepass dylandepass deleted the feat-235 branch August 28, 2024 01:45
rofe pushed a commit that referenced this pull request Aug 28, 2024
# [1.30.0](v1.29.0...v1.30.0) (2024-08-28)

### Features

* support cross origin request to tools and labs ([#236](#236)) ([314e2b8](314e2b8))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify response headers to include the ACAO header if user is logged in and using tools or labs
2 participants