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

[🚀 ESL Popup] add position-origin attribute #2747

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

dshovchko
Copy link
Collaborator

@dshovchko dshovchko commented Nov 6, 2024

Added a new attribute position-origin="outer|inner" that allows or disallows to overlap of the trigger by a popup window.

sshot_2024-11-06-12-47-20

Closes: #2746

@dshovchko dshovchko added feature New feature needs review The PR is waiting for review labels Nov 6, 2024
@dshovchko dshovchko self-assigned this Nov 6, 2024
@dshovchko dshovchko changed the title Feat/esl popup position origin [:rocket: ESL Popup] add position-origin attribute Nov 6, 2024
@dshovchko dshovchko changed the title [:rocket: ESL Popup] add position-origin attribute [🚀 ESL Popup] add position-origin attribute Nov 6, 2024
break;
case 'right':
x = inner.right;
x = hasInnerOrigin ? inner.x : inner.right;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note: probably we can use left/right key and some generic function for invertion in future

@@ -401,10 +407,11 @@ export class ESLPopup extends ESLToggleable {
const triggerRect = Rect.from(this.activator).shift(window.scrollX, window.scrollY);
const {containerRect} = this;

const innerMargin = this._offsetTrigger + arrowRect.width / 2;
const innerMargin = this._offsetTrigger + (this.positionOrigin === 'inner' ? 0 : arrowRect.width / 2);
Copy link
Collaborator

@ala-n ala-n Nov 6, 2024

Choose a reason for hiding this comment

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

Do we need? Practically there is no limitation to have arrow for inner behaviour

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not a limitation. This is just a position adjustment.

Copy link
Collaborator

@ala-n ala-n left a comment

Choose a reason for hiding this comment

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

We need to think of optimization of these utils...

@dshovchko
Copy link
Collaborator Author

We need to think of optimization of these utils...

I'm going to create a few issues to this

src/modules/esl-popup/README.md Outdated Show resolved Hide resolved
@ala-n ala-n merged commit dfde8f6 into main-beta Nov 7, 2024
7 checks passed
@ala-n ala-n deleted the feat/esl-popup-position-origin branch November 7, 2024 13:45
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2024
@ala-n
Copy link
Collaborator

ala-n commented Nov 11, 2024

🎉 This PR is included in version 5.0.0-beta.39 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature needs review The PR is waiting for review released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants