[MM-60605] Fix the Download button being hidden on Windows/Linux #3148

devinbinnie · 2024-09-19T15:35:31Z

Summary

When we made the change to switch to the titleBarOverlay for Windows and Linux, the Downloads dropdown button has disappeared. This is because it was hidden behind the new overlay.

This PR adds a span to force the Downloads dropdown in front of the title bar buttons.

NOTE: Will have to update this when cherry-picking to v5.9, as Windows doesn't use these new buttons on v5.9.

Ticket Link

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

Fix the Download button being hidden on Windows/Linux

…and Windows

marianunez · 2024-09-19T19:18:18Z

NOTE: Will have to update this when cherry-picking to v5.9, as Windows doesn't use these new buttons on v5.9.

What are these new buttons? Just trying to get more context here

devinbinnie · 2024-09-19T19:23:59Z

NOTE: Will have to update this when cherry-picking to v5.9, as Windows doesn't use these new buttons on v5.9.

What are these new buttons? Just trying to get more context here

They're not really new - just replacements for what we had, using native Electron APIs instead of the custom ones we had ourselves.

marianunez

Anything we can test to avoid this issue in the future?

marianunez · 2024-09-19T19:39:17Z

src/renderer/components/MainPage.tsx

+                    {window.process.platform !== 'darwin' &&
+                        <span style={{width: `${window.innerWidth - (window.navigator.windowControlsOverlay?.getTitlebarAreaRect().width ?? 0)}px`}}/>
+                    }


Would there be any overlap in full screen due to the condition below?

Ugh...why does full screen mode have to exist...
Seems like it's causing some weird behaviour, i'll fix that.

Okay I've fixed it for Windows and "fixed" it for Linux.
Unfortunately full screen on Linux really doesn't work very well, so we'll need to address that separately. See: #3143

devinbinnie · 2024-09-19T19:39:22Z

Anything we can test to avoid this issue in the future?

If we had front-end tests, absolutely. Never was able to add those.
Add to the pile of things to do for Desktop App 🙃

marianunez · 2024-09-19T21:33:20Z

src/renderer/components/MainPage.tsx

+            } else {
+                titleBarSpacing = (<span style={{width: `${window.innerWidth - (window.navigator.windowControlsOverlay?.getTitlebarAreaRect().width ?? 0)}px`}}/>);


So, does this mean that Linux doesn't need this special case for full screen?

At this point, no. I spent some time trying to work around it but full screen under Linux refuses to behave nicely. So i'm going to create a separate PR that removes full screen support from Linux.

marianunez · 2024-09-19T21:33:57Z

src/renderer/components/MainPage.tsx

+            if (this.state.fullScreen && window.process.platform !== 'linux') {
+                titleBarSpacing = (
+                    <div
+                        className={`button full-screen-button${this.props.darkMode ? ' darkMode' : ''}`}
+                        onClick={this.handleExitFullScreen}
+                    >
+                        <i className='icon icon-arrow-collapse'/>
+                    </div>
+                );


In full-screen does the download button is shown with this case for Windows?

Yes, it shows correctly

mattermost-build · 2024-09-23T12:41:42Z

Cherry pick is scheduled.

mattermost-build · 2024-09-23T12:41:44Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
From github.com:mattermost/desktop
 * [new branch]        MM-60201    -> upstream/MM-60201
   820ef4d4..9db56c50  MM-60605    -> upstream/MM-60605
 * [new branch]        disable_fullscreen_linux -> upstream/disable_fullscreen_linux
   b4eeabb3..3b179c7e  master      -> upstream/master
   97c7331c..0e49afd5  release-5.9 -> upstream/release-5.9
 * [new branch]        sbom        -> upstream/sbom
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-desktop-#3148-upstream-release-5.9-1727095303
Switched to a new branch 'automated-cherry-pick-of-desktop-#3148-upstream-release-5.9-1727095303'
Branch 'automated-cherry-pick-of-desktop-#3148-upstream-release-5.9-1727095303' set up to track remote branch 'release-5.9' from 'upstream'.

+++ About to attempt cherry pick of PR #3148 with merge commit 3b179c7ec2d9936b0079874d30092005c44d0276.

Auto-merging src/renderer/components/MainPage.tsx
CONFLICT (content): Merge conflict in src/renderer/components/MainPage.tsx
Auto-merging src/renderer/css/index.css
CONFLICT (content): Merge conflict in src/renderer/css/index.css
Auto-merging src/types/window.ts
error: could not apply 3b179c7e... [MM-60605] Fix the Download button being hidden on Windows/Linux (#3148)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU src/renderer/components/MainPage.tsx
UU src/renderer/css/index.css
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

* [MM-60605] Fix missing downloads/developer mode icon hidden on Linux and Windows * Disable for mac * Fix lint * Fix misalignment on Windows * "fix" linux * Return to inline version, ignore Linux

…) (#3154) * [MM-60605] Fix missing downloads/developer mode icon hidden on Linux and Windows * Disable for mac * Fix lint * Fix misalignment on Windows * "fix" linux * Return to inline version, ignore Linux

devinbinnie added 2 commits September 19, 2024 11:25

[MM-60605] Fix missing downloads/developer mode icon hidden on Linux …

d409890

…and Windows

Disable for mac

1905067

devinbinnie added the 2: Dev Review Requires review by a core committer label Sep 19, 2024

devinbinnie added this to the v5.9.0 milestone Sep 19, 2024

devinbinnie requested review from marianunez and Rajat-Dabade September 19, 2024 15:35

mm-cloud-bot added the release-note label Sep 19, 2024

Fix lint

820ef4d

marianunez reviewed Sep 19, 2024

View reviewed changes

devinbinnie added 2 commits September 19, 2024 16:33

Fix misalignment on Windows

40fdb40

"fix" linux

080766a

devinbinnie requested a review from marianunez September 19, 2024 21:04

marianunez reviewed Sep 19, 2024

View reviewed changes

amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Sep 19, 2024

Return to inline version, ignore Linux

9db56c5

devinbinnie requested a review from marianunez September 20, 2024 18:13

marianunez approved these changes Sep 20, 2024

View reviewed changes

Rajat-Dabade approved these changes Sep 21, 2024

View reviewed changes

devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Sep 23, 2024

devinbinnie merged commit 3b179c7 into master Sep 23, 2024
17 of 18 checks passed

devinbinnie deleted the MM-60605 branch September 23, 2024 12:41

devinbinnie mentioned this pull request Sep 23, 2024

[MM-60605] Cherrypick of (#3148) to release-5.9 #3154

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-60605] Fix the Download button being hidden on Windows/Linux #3148

[MM-60605] Fix the Download button being hidden on Windows/Linux #3148

devinbinnie commented Sep 19, 2024

marianunez commented Sep 19, 2024

devinbinnie commented Sep 19, 2024

marianunez left a comment

marianunez Sep 19, 2024

devinbinnie Sep 19, 2024

devinbinnie Sep 19, 2024

devinbinnie commented Sep 19, 2024

marianunez Sep 19, 2024

devinbinnie Sep 20, 2024

marianunez Sep 19, 2024

devinbinnie Sep 20, 2024

mattermost-build commented Sep 23, 2024

mattermost-build commented Sep 23, 2024 •

edited by jira bot

Loading

		} else {
		titleBarSpacing = (<span style={{width: `${window.innerWidth - (window.navigator.windowControlsOverlay?.getTitlebarAreaRect().width ?? 0)}px`}}/>);

[MM-60605] Fix the Download button being hidden on Windows/Linux #3148

[MM-60605] Fix the Download button being hidden on Windows/Linux #3148

Conversation

devinbinnie commented Sep 19, 2024

Summary

Ticket Link

marianunez commented Sep 19, 2024

devinbinnie commented Sep 19, 2024

marianunez left a comment

Choose a reason for hiding this comment

marianunez Sep 19, 2024

Choose a reason for hiding this comment

devinbinnie Sep 19, 2024

Choose a reason for hiding this comment

devinbinnie Sep 19, 2024

Choose a reason for hiding this comment

devinbinnie commented Sep 19, 2024

marianunez Sep 19, 2024

Choose a reason for hiding this comment

devinbinnie Sep 20, 2024

Choose a reason for hiding this comment

marianunez Sep 19, 2024

Choose a reason for hiding this comment

devinbinnie Sep 20, 2024

Choose a reason for hiding this comment

mattermost-build commented Sep 23, 2024

mattermost-build commented Sep 23, 2024 • edited by jira bot Loading

mattermost-build commented Sep 23, 2024 •

edited by jira bot

Loading