Skip to content

fix(shell): fix chat being active in the background of the shell #18088

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

Open
wants to merge 1 commit into
base: feat/expose-last-message
Choose a base branch
from

Conversation

jrainville
Copy link
Member

What does the PR do

Fixes #18059

Fixes the issue by putting the Shell as part of the sections themselves. that way we ensure that no other section is active in the background for no reason. It simplifies the code a little since we no longer need a custom shell button

Affected areas

  • main module (added the new section and removed the old code to set a section as active)
  • AppMain (moved the Shell in the StackLayout)

Architecture compliance

Screenshot of functionality (including design for comparison)

chat-in-background-shell.webm

Impact on end user

Fixes the issue of chat messages being marked as read even if not shown to the user. Nothing in the background when in the shell, so no risk of side effects

How to test

  • Use the shell as usual

Risk

Tick one:

  • Low risk: 2 devs MUST perform testing as specified above and attach their results as comments to this PR before merging.
  • High risk: QA team MUST perform additional testing in the specified affected areas before merging.

@jrainville jrainville requested review from micieslak, caybro, alexjba, noeliaSD and a team as code owners June 6, 2025 18:22
@@ -167,7 +167,6 @@ QtObject:

proc activeSectionSet*(self: View, item: SectionItem) =
self.activeSection.setActiveSectionData(item)
self.activeSectionChanged()
Copy link
Member Author

Choose a reason for hiding this comment

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

This was useless. It just made it so that we called the update twice.

@@ -55,8 +55,7 @@ SplitView {
}

Control {
SplitView.minimumWidth: 78
SplitView.preferredWidth: 78
SplitView.preferredWidth: !!leftPanel && leftPanel.visible ? leftPanel.width : 0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to keep the previous behaviour of the shell being full screen. IMO, it looks fine when the tabbar is still shown when the Shell is visible, but we can easily revert.

@@ -51,7 +51,8 @@ Control {
spacing: Theme.bigPadding

function focusSearch() {
searchField.forceActiveFocus()
// Need to use Qt.callLater to ensure the focus is set after the component is fully loaded
Qt.callLater(() => searchField.forceActiveFocus())
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the Shell is shown at the start for real now (loaded by the main module), it loaded too fast and the focus didn't apply

@@ -1819,6 +1808,93 @@ Item {
// If we ever change stack layout component order we need to updade
// Constants.appViewStackIndex accordingly

Loader {
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't really change anything here, just moved it in the StackLayout

@status-im-auto
Copy link
Member

status-im-auto commented Jun 6, 2025

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ fc218ea #1 2025-06-06 18:29:23 ~6 min tests/nim 📄log
✔️ fc218ea #1 2025-06-06 18:32:44 ~10 min macos/aarch64 🍎dmg
✔️ fc218ea #1 2025-06-06 18:33:08 ~10 min tests/ui 📄log
✔️ fc218ea #1 2025-06-06 18:36:29 ~14 min linux/x86_64 📦tgz
✔️ fc218ea #1 2025-06-06 18:43:39 ~21 min macos/x86_64 🍎dmg
✔️ fc218ea #1 2025-06-06 18:45:20 ~22 min windows/x86_64 💿exe
✖️ 006e663 #2 2025-06-09 15:53:33 ~6 min tests/nim 📄log
✔️ 006e663 #2 2025-06-09 15:57:12 ~9 min macos/aarch64 🍎dmg
✔️ 006e663 #2 2025-06-09 15:57:41 ~10 min tests/ui 📄log
✔️ 006e663 #2 2025-06-09 16:01:07 ~13 min linux/x86_64 📦tgz
✔️ 006e663 #2 2025-06-09 16:04:57 ~17 min macos/x86_64 🍎dmg
✔️ 006e663 #2 2025-06-09 16:13:10 ~25 min windows/x86_64 💿exe

@jrainville jrainville force-pushed the feat/expose-last-message branch from 96fb531 to f73e740 Compare June 9, 2025 14:59
@jrainville jrainville requested a review from a team as a code owner June 9, 2025 14:59
@jrainville jrainville force-pushed the feat/expose-last-message branch from f73e740 to 98149ad Compare June 9, 2025 15:46
Fixes #18059

Fixes the issue by putting the Shell as part of the sections themselves.
that way we ensure that no other section is active in the background for no reason.
It simplifies the code a little since we no longer need a custom shell button
@jrainville jrainville force-pushed the fix/active-chat-badge-in-shell branch from fc218ea to 006e663 Compare June 9, 2025 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants