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(dialogs): Remove history hacks in dialog #1041

Merged
merged 6 commits into from
Mar 13, 2024

Conversation

pladaria
Copy link
Member

@pladaria pladaria commented Mar 4, 2024

Remove history hacks from dialog components to avoid interfering with similar hacks in apps.

  • Remove history push and back navigation from dialog components
  • It still listens to 'popstate' event to force a dismiss

Verification of this branch against the webapp brach that enables related skipped tests:
https:/Telefonica/webapp/actions/runs/8232821830 (multiple successful runs)

TODO:

  • Perform some manual testing inside the app before merging (just to make sure the native dialogs work as expected with the webapp hacks and the new implementation)

Discarded options:

  • use a modal <dialog> element. Discarded because it interferes with other elements with overlays (like selects or tooltips), fixing that would require a large refactor and the previous implementation cannot be removed because <dialog> is not supported before safari 15.4. Additionally, the Close Watcher with back seems to be broken in chrome (it works using ESC).
  • keep the hack but try to workaround issues with webapp implementations. That is a hack to workaround other hacks. Better avoid this kind of madness.

Copy link

github-actions bot commented Mar 4, 2024

Size stats

master this branch diff
Total JS 10.6 MB 10.6 MB -415 B
JS without icons 1.91 MB 1.91 MB -415 B
Lib overhead 49.4 kB 49.4 kB 0 B
Lib overhead (gzip) 13.4 kB 13.4 kB 0 B

Copy link

github-actions bot commented Mar 4, 2024

Accessibility report
✔️ No issues found

ℹ️ You can run this locally by executing yarn audit-accessibility.

Copy link

github-actions bot commented Mar 4, 2024

Deploy preview for mistica-web ready!

✅ Preview
https://mistica-njfl6gbv5-tuentisre.vercel.app

Built with commit 189e0ed.
This pull request is being automatically deployed with vercel-action

@pladaria pladaria changed the title WEB-1761 remove history hacks in dialog feat(dialogs): Remove history hacks in dialog Mar 4, 2024
Copy link

github-actions bot commented Mar 4, 2024

Screenshot tests report

✔️ All passing

@pladaria pladaria force-pushed the pladaria/WEB-1761_remove-history-hacks-from-dialogs branch from bec779c to 8830f25 Compare March 7, 2024 09:52
@brtbrt
Copy link
Contributor

brtbrt commented Mar 12, 2024

weren't you experiencing some problem with our restrictNavigation hook? i.e. the three "back" actions to navigate back

@pladaria
Copy link
Member Author

weren't you experiencing some problem with our restrictNavigation hook? i.e. the three "back" actions to navigate back

I guess my previous implementation had issues. I rewrote this from scratch and it works as expected :)

@pladaria pladaria added this pull request to the merge queue Mar 13, 2024
Merged via the queue into master with commit 2a48cb4 Mar 13, 2024
11 checks passed
@pladaria pladaria deleted the pladaria/WEB-1761_remove-history-hacks-from-dialogs branch March 13, 2024 14:59
tuentisre pushed a commit that referenced this pull request Mar 13, 2024
# [14.46.0](v14.45.0...v14.46.0) (2024-03-13)

### Bug Fixes

* **CSS:** prevent browser default behaviours when long pressing or dragging links ([#1045](#1045)) ([717343d](717343d))
* **HighlightedCard:** improve optional types of title and description, make one mandatory ([#1039](#1039)) ([a818ffa](a818ffa))
* **Logo:** old version of Tu logo ([#1036](#1036)) ([0dc09e2](0dc09e2))
* **Switch:** iOS background in dark mode ([#1037](#1037)) ([a77c42d](a77c42d))

### Features

* **dialogs:** Remove history hacks in dialog ([#1041](#1041)) ([2a48cb4](2a48cb4))
* **IconButton:** update internal usages in Mistica components ([#1044](#1044)) ([12bf970](12bf970))
* **Icons:** New vivo icons ([#1038](#1038)) ([d070a25](d070a25))
* **Input fields:** update iconButton in all fields ([#1042](#1042)) ([062c0bd](062c0bd))
* **Naked & Small Naked Card:** Inner padding right in card content ([#1040](#1040)) ([d97ea17](d97ea17))
* **Rows:** add icon button as control ([#1030](#1030)) ([4d47e8f](4d47e8f))
* **ToggleIconButton:** create component ([#1034](#1034)) ([f4b9c6e](f4b9c6e))
* **Video:** `stop` method. To stop video and show the poster (if available) ([#1035](#1035)) ([e21998f](e21998f))
@tuentisre
Copy link
Collaborator

🎉 This PR is included in version 14.46.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants