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

fix: Multi-step modals closing unexpectedly #33158

Merged
merged 5 commits into from
Aug 28, 2024

Conversation

dougfabris
Copy link
Member

@dougfabris dougfabris commented Aug 26, 2024

Introduced here: #32482

Proposed changes (including videos or screenshots)

With the introduction of the onClose call in the useEffect cleanup function in GenericModal, multi-step modals using GenericModal started to close unexpectedly. This PR aims to remove the usage of the GenericModal in favor of GenericModalSkeleton to solve this issue and keep the loading consistency.

Issue(s)

Steps to test or reproduce

Further comments

CORE-660

Copy link
Contributor

dionisio-bot bot commented Aug 26, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 6.13.0, but it targets 6.12.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Aug 26, 2024

🦋 Changeset detected

Latest commit: 320a8dd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.33%. Comparing base (9ef09d8) to head (320a8dd).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #33158   +/-   ##
========================================
  Coverage    59.33%   59.33%           
========================================
  Files         2547     2546    -1     
  Lines        63258    63247   -11     
  Branches     14221    14220    -1     
========================================
- Hits         37531    37529    -2     
+ Misses       23025    23016    -9     
  Partials      2702     2702           
Flag Coverage Δ
unit 75.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@dougfabris dougfabris added this to the 6.13 milestone Aug 26, 2024
@dougfabris dougfabris force-pushed the fix/multistep-modal-closing branch 2 times, most recently from 9c825eb to 930bf9c Compare August 27, 2024 17:36
@dougfabris dougfabris marked this pull request as ready for review August 27, 2024 21:25
@dougfabris dougfabris requested a review from a team as a code owner August 27, 2024 21:25
@jessicaschelly jessicaschelly added the stat: QA assured Means it has been tested and approved by a company insider label Aug 28, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Aug 28, 2024
@kodiakhq kodiakhq bot merged commit 5cbbb45 into develop Aug 28, 2024
47 checks passed
@kodiakhq kodiakhq bot deleted the fix/multistep-modal-closing branch August 28, 2024 14:01
@scuciatto
Copy link
Member

/patch

Copy link
Contributor

dionisio-bot bot commented Aug 28, 2024

    Sorry, I couldn't do that backport because of conflicts. Could you please solve them?
    
    you can do so by running the following commands:
git fetch
git checkout backport-6.11.2-33158
git cherry-pick 5cbbb4585146fe28474fe89789172218abd266f8
// solve the conflict
git push

after that just run /patch again

@dougfabris
Copy link
Member Author

/patch

Copy link
Contributor

dionisio-bot bot commented Aug 28, 2024

Pull request #33178 added to Project: "Patch 6.11.2"

gabriellsh added a commit that referenced this pull request Sep 2, 2024
…ove/threadMetrics

* 'develop' of github.com:RocketChat/Rocket.Chat: (49 commits)
  feat: add `sidepanelNavigation` to Feature preview list (