Fix broken macOS check by luk1337 · Pull Request #690 · Syncplay/syncplay · GitHub
Skip to content

Fix broken macOS check#690

Merged
Et0h merged 2 commits into
Syncplay:masterfrom
luk1337:luk/split
Sep 30, 2024
Merged

Fix broken macOS check#690
Et0h merged 2 commits into
Syncplay:masterfrom
luk1337:luk/split

Conversation

@luk1337

@luk1337 luk1337 commented Jul 15, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@luk1337

luk1337 commented Jul 15, 2024

Copy link
Copy Markdown
Contributor Author

@luk1337 luk1337 changed the title Fix minimujm heights for split labels Fix minimum heights for split labels Jul 15, 2024
@selurvedu

Copy link
Copy Markdown

@luk1337 you posted the same screenshot twice.

@luk1337

luk1337 commented Aug 1, 2024

Copy link
Copy Markdown
Contributor Author

@luk1337 you posted the same screenshot twice.

no, look at the highlighted part:
image

@selurvedu

Copy link
Copy Markdown

Oooh, riiight. That's so hard to notice if you don't know where to look. Sorry for the noise. 😅

@luk1337

luk1337 commented Aug 1, 2024

Copy link
Copy Markdown
Contributor Author

Oooh, riiight. That's so hard to notice if you don't know where to look. Sorry for the noise. 😅

np, you can also notice that SSL icon has different style ^^

@Et0h

Et0h commented Sep 28, 2024

Copy link
Copy Markdown
Contributor

The fix looks good on Windows and Linux, but we have not tested it on macOS and the first change seems to be targeted at that OS. Have you tested your first change on macOS to confirm it fixes an issue rather than introducing one?

@luk1337

luk1337 commented Sep 28, 2024

Copy link
Copy Markdown
Contributor Author

The fix looks good on Windows and Linux, but we have not tested it on macOS and the first change seems to be targeted at that OS. Have you tested your first change on macOS to confirm it fixes an issue rather than introducing one?

ok, I tested it on macOS and this was breaking it, so I just restored it to how it was before.

if isMacOS():
    window.outputlabel.setMinimumHeight(21)
else:
    window.outputlabel.setMinimumHeight(27)

And with just fixing broken macOS check, this change now only affects non-macOS.

@luk1337 luk1337 changed the title Fix minimum heights for split labels Fix broken macOS check Sep 28, 2024
@luk1337

luk1337 commented Sep 28, 2024

Copy link
Copy Markdown
Contributor Author

The fix looks good on Windows and Linux, but we have not tested it on macOS and the first change seems to be targeted at that OS. Have you tested your first change on macOS to confirm it fixes an issue rather than introducing one?

ok, I tested it on macOS and this was breaking it, so I just restored it to how it was before.

if isMacOS():
    window.outputlabel.setMinimumHeight(21)
else:
    window.outputlabel.setMinimumHeight(27)

And with just fixing broken macOS check, this change now only affects non-macOS.

image

@Et0h

Et0h commented Sep 30, 2024

Copy link
Copy Markdown
Contributor

@Et0h Et0h merged commit 4f1fe8d into Syncplay:master Sep 30, 2024
@luk1337 luk1337 deleted the luk/split branch September 30, 2024 18:52
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.

3 participants