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

Don't preload goal settings modal to avoid issues with opening in edit mode #5031

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

zoldar
Copy link
Contributor

@zoldar zoldar commented Jan 29, 2025

This PR fixes an issue where, on a fresh view load, when trying to open edit dialog for an existing goal, the form is not loaded properly. The preloading optimisation (meant load modal contents in the background in a state defined on page load - in this case, an empty form) breaks it, as first open does not force contents to be reloaded when they should. The modal preopen routine can probably be improved to account for this case with a switch of sorts but currently this is the simplest solution for that particular case.

All the other current uses of the modal component do not have "edit" state currently (Funnel settings still rely on a custom-made solution).

@zoldar zoldar requested review from aerosol and apata January 29, 2025 15:09
@zoldar zoldar added the preview label Jan 29, 2025
Copy link

Preview environment👷🏼‍♀️🏗️
PR-5031

@zoldar zoldar marked this pull request as draft January 29, 2025 15:21
@zoldar
Copy link
Contributor Author

zoldar commented Jan 29, 2025

Converting this PR back to draft as there's a test failing still.

@zoldar zoldar force-pushed the fix-goal-settings-edit-modal-on-first-load branch 2 times, most recently from 6de98c8 to e13783e Compare January 30, 2025 08:52
@zoldar zoldar force-pushed the fix-goal-settings-edit-modal-on-first-load branch from e13783e to eb2b167 Compare January 30, 2025 09:17
@zoldar zoldar marked this pull request as ready for review January 30, 2025 09:22
@zoldar
Copy link
Contributor Author

zoldar commented Jan 30, 2025

It's finally fixed and ready for review.

@zoldar zoldar added this pull request to the merge queue Jan 30, 2025
Merged via the queue into master with commit 86f3919 Jan 30, 2025
8 checks passed
@zoldar zoldar deleted the fix-goal-settings-edit-modal-on-first-load branch January 30, 2025 11:06
RobertJoonas pushed a commit that referenced this pull request Jan 30, 2025
…t mode (#5031)

* Don't preload goal settings modal to avoid issues with opening in edit mode

* Remove test which is no longer holding ture

* Still enable preload for tests, as it's needed

* Fix and move test env switch inside the modal component

* Fix for release build
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.

2 participants