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.

Sign up for GitHub

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

Jump to bottom

fix: pop confirm position bugs #885

Merged
merged 6 commits into from
Oct 20, 2023
Merged

fix: pop confirm position bugs #885

merged 6 commits into from
Oct 20, 2023

Conversation

allan-chagas-brisa
Copy link
Contributor

@allan-chagas-brisa allan-chagas-brisa commented Oct 19, 2023 •
edited
Loading

Issue Number

fix #857 #858

Proposed Changes

  • Added a call to the ngOnDestroy method on the directive to close whenever the host is destroyed;
  • minor changes on the popconfirm position calculation, to ensure the arrow and the popconfirm itself are located correctly;
  • added stories to visualize these changes.

How to Test

yarn test popconfirm

Screenshots

Gravacao.de.tela.de.19-10-2023.10.01.22.webm

View Storybook

Storybook

Compliance

  • I have verified that this change complies with our code and contribution policies.
  • I have verified that this change does not cause regressions and does not affect other parts of the code.

@allan-chagas-brisa allan-chagas-brisa linked an issue Oct 19, 2023 that may be closed by this pull request
fix: ion-popconfirm on a ion-button inside a ion-modal doesn't appear correctly #857
Closed
@allan-chagas-brisa allan-chagas-brisa self-assigned this Oct 19, 2023
@allan-chagas-brisa allan-chagas-brisa marked this pull request as ready for review October 19, 2023 13:34
danilo-moreira-brisa
danilo-moreira-brisa approved these changes Oct 19, 2023
View reviewed changes
Copy link
Contributor

@danilo-moreira-brisa danilo-moreira-brisa left a comment

Choose a reason for hiding this comment

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

Good job!

@allan-chagas-brisa allan-chagas-brisa linked an issue Oct 20, 2023 that may be closed by this pull request
fix: ion-popconfirm remains open even if the parent component is destroyed #858
Closed
@allan-chagas-brisa allan-chagas-brisa merged commit 43a724e into main Oct 20, 2023
3 checks passed
@allan-chagas-brisa allan-chagas-brisa deleted the 857-858-fix-ion-popconfirm branch October 20, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wermeson-lopes-brisa wermeson-lopes-brisa wermeson-lopes-brisa approved these changes

@danilo-moreira-brisa danilo-moreira-brisa danilo-moreira-brisa approved these changes

@iurynogueira iurynogueira Awaiting requested review from iurynogueira

@lucas-alcantara-brisa lucas-alcantara-brisa Awaiting requested review from lucas-alcantara-brisa

@deeved-hiuston-brisa deeved-hiuston-brisa Awaiting requested review from deeved-hiuston-brisa

@pedro-martins-brisa pedro-martins-brisa Awaiting requested review from pedro-martins-brisa

Assignees

@allan-chagas-brisa allan-chagas-brisa

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@allan-chagas-brisa @danilo-moreira-brisa @wermeson-lopes-brisa