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(suite): fix draggable window header #16437

Merged
merged 2 commits into from
Jan 27, 2025
Merged

Conversation

jvaclavik
Copy link
Contributor

@jvaclavik jvaclavik commented Jan 17, 2025

Description

Related Issue

Resolve

Screenshots:

Red draggable zone is not visible anymore.

image image image image

@bosomt
Copy link
Contributor

bosomt commented Jan 27, 2025

QA OK

macOS

  • Suite version: desktop 25.2.0 (16d98ba)
  • is draggable everywhere except at ⚙️ icon , connected or disconnected device

Win11

  • works as expected, in WIn env there is app bar that you always can grab and drag

Mint

  • same for Linux, it has dedicated app bar that can be grabbed and dragged anytime

@jvaclavik jvaclavik force-pushed the fix-draggable-window-header branch from 5af384d to cabe8e8 Compare January 27, 2025 07:29
@jvaclavik jvaclavik marked this pull request as ready for review January 27, 2025 08:04
@jvaclavik jvaclavik requested a review from vojtatranta January 27, 2025 08:48

if (!isVisible || !isMac || !isDesktopApp) return children;

return <FixForNotBeingAbleToDragWindow />;
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT (meaning nitpick, if won't added, it's okay): I would add a comment here explain of why this fix is needed in these conditions, what's the root of it.

@vojtatranta
Copy link
Contributor

NIT: Overall opinion: the problem with fixes like this is that these are dependent on multiple changes in the codebase that are unrelated.
I can think of a test cast that would resolve it - you'd need to render the whole react tree and make a one test case with multiple expect() that would check that the styles and components are where they are.

I am not sure if the codebase is prepared for that. So I would at least add comments to the changes of why they are there.

@jvaclavik jvaclavik force-pushed the fix-draggable-window-header branch from cabe8e8 to d30bde9 Compare January 27, 2025 09:10
@jvaclavik jvaclavik merged commit 6123369 into develop Jan 27, 2025
32 checks passed
@jvaclavik jvaclavik deleted the fix-draggable-window-header branch January 27, 2025 09:30
@STew790 STew790 added the QA OK Issue passed QA without any blocker label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA OK Issue passed QA without any blocker
Projects
Status: ✅ Approved
Development

Successfully merging this pull request may close these issues.

5 participants