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

[MM-62001] Calls: switch tab on actions requiring focus #3235

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

streamer45
Copy link
Contributor

Summary

For calls widget actions requiring focus on the main channel view (e.g., channel link click, showing screen sharing selection modal), we were not handling the possibility that the tab was something other than messaging. We fix this by switching to the expected tab.

Ticket Link

https://mattermost.atlassian.net/browse/MM-62001

Release Note

Fixed an issue preventing the screen sharing selection modal to show when the app was focused on a different tab (e.g. Playbooks, Boards)

@streamer45 streamer45 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Dec 2, 2024
@streamer45 streamer45 self-assigned this Dec 2, 2024
Copy link
Member

@cpoile cpoile left a comment

Choose a reason for hiding this comment

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

Looks clean, thanks! And thanks for the great tests!

@devinbinnie devinbinnie removed the 2: Dev Review Requires review by a core committer label Dec 3, 2024
@streamer45
Copy link
Contributor Author

@DHaussermann Left some test steps in the ticket. Let me know if you need more info.

@DHaussermann DHaussermann added Build Apps for PR Builds signed builds for testing and removed Build Apps for PR Builds signed builds for testing labels Dec 6, 2024
@DHaussermann
Copy link

@streamer45 I have done a round of testing on this and the issue as described is resolved.
From the widget, the share is working and will flip to channels if needed to show you the screen share step. All other actions from the modal work and switch to channel context if needed (settings for example)

But I think something might be going wrong here. The widget aside, I see an issue where the screen share option is no longer working from the expanded view. Not 100% sure this was in scope but, I can't repro the issue on our production build. I don't just mean for switching tabs if needed. For me the share button no longer works from any tab.

Can you please see if you can repo this? (I have only tried on Mac)

  • Install the build from the pipeline on this PR 5.11.0-develop.1
  • Login to a server (I tried mutiple) and start a call
  • Stay on the channels tab (optional)
  • From the widget open the expanded view
  • From the expanded view click the screen share
    Observed Nothing happens. The button seems non-functional.
    I don't have more details because I can seem to got to the expanded view in dev tools. I only see option for the main window and the widget. So I have no data from network tab or console.

@streamer45
Copy link
Contributor Author

@DHaussermann It sounds like you may be hitting https://mattermost.atlassian.net/browse/MM-62241, which was reported by @matthewbirtch as well during testing of #3253

I'll look at that separately since it's not related to these PRs as far as I can tell.

@DHaussermann
Copy link

Ah of course! Thanks @streamer45! This does seem like the issue I'm seeing. It seemed strange and as they both relate to screen share it seemed plausible it was from this PR 🙃.

@streamer45
Copy link
Contributor Author

Absolutely!

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Confirmed issue is resolved. Screen share from widget is functional and will flip to channels tab if needed
  • Confirmed other options that need to flip to channels tab such as setting are also functional if you're on a different tab
  • Regression tested other action in the widget that do not require flipping tabs

LGTM!

Issue with screen share via expanded view will be addressed separately.

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Dec 13, 2024
@streamer45 streamer45 merged commit c2bfead into master Dec 13, 2024
56 checks passed
@streamer45 streamer45 deleted the MM-62001 branch December 13, 2024 21:11
@amyblais amyblais added this to the v5.11.0 milestone Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants