Skip to content

Conversation

@ShahzaibIbrahim
Copy link
Contributor

@ShahzaibIbrahim ShahzaibIbrahim commented Oct 29, 2024

In the UI Preferences -> General -> Appearance, there is a new HiDPI setting that could be checked to enable the monitor-specific scaling of the UI. This check will also enable Edge browser by default. This is an experimental feature to test the multi-monitor HiDPI support before it is actually released and set by default. For now, this preference is disabled by default.

Note: This preference check is only available in Windows

image

When changing the preference, the dialog will appear to prompt the restart.

image

@laeubi
Copy link
Contributor

laeubi commented Oct 29, 2024

As we have ways to know the platform, maybe only show it when running on windows?

Also either make PerMonitorV2 a link to some documentation or remove implementation details from the description.

@ShahzaibIbrahim
Copy link
Contributor Author

As we have ways to know the platform, maybe only show it when running on windows?

@laeubi Forgot to mention but yes, we are only showing it in Windows. I have updated the description.

@laeubi
Copy link
Contributor

laeubi commented Oct 29, 2024

If it is only shown on windows, then I don't think it needs to be mentioned as a "Windows only" in the text.

@BeckerWdf
Copy link
Member

Also either make PerMonitorV2 a link to some documentation or remove implementation details from the description.

One could also add such details into the documentation page for this preference page.

@ShahzaibIbrahim
Copy link
Contributor Author

@BeckerWdf @laeubi for now I have removed implementation details from the description. I have asked if we have documentation page for this right now. If yes, then I will link it.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 29, 2024

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 43m 42s ⏱️ - 10m 43s
 7 724 tests ±0   7 495 ✅ ±0  228 💤 ±0  1 ❌ ±0 
24 333 runs  ±0  23 585 ✅ ±0  747 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit a2bbf11. ± Comparison against base commit 83c128c.

♻️ This comment has been updated with latest results.

Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

It works pretty well, I only have some minor comments about naming/typos and one important detail about preserving the selected browser when the new preference is set to false.

@ShahzaibIbrahim ShahzaibIbrahim force-pushed the master-134 branch 2 times, most recently from ffccae6 to a05574d Compare October 30, 2024 14:56
@fedejeanne fedejeanne dismissed their stale review October 31, 2024 05:57

My changes have been applied. Rebasing.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Great to finally have a proper preference for enabling the enhanced HiDPI support on Windows, thanks!

Still, in the current state, this PR introduces a regression and needs some revision regarding design and naming. See my detailed comments.

@ShahzaibIbrahim ShahzaibIbrahim force-pushed the master-134 branch 2 times, most recently from c641c89 to 351988b Compare October 31, 2024 13:10
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Regarding functionality, this looks good now (except for the case that the options do not show up anymore because of a faulty conditions).

Some minor code style comments are remaining.

@HeikoKlare HeikoKlare changed the title Adding experimental preference to enable update on runtime behavior [win] Adding experimental preference to enable update on runtime behavior Oct 31, 2024
In the UI Preferences -> General -> Appearance, there is a new HiDPI
setting that could be checked to enable the monitor-specific scaling of
the UI. This check will also enable Edge browser by default. This is an
experimental feature to test the multi-monitor HiDPI support before it
is actually released and set by default. For now, this preference is
disabled by default.
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

From my side, this also looks good now. Thank you!

@HeikoKlare
Copy link
Contributor

Failing test is unrelated/already documented: #370

@HeikoKlare HeikoKlare merged commit c5928bb into eclipse-platform:master Oct 31, 2024
15 of 17 checks passed
@HeikoKlare HeikoKlare added the noteworthy Noteworthy feature label Nov 1, 2024
ShahzaibIbrahim added a commit to ShahzaibIbrahim/www.eclipse.org-eclipse that referenced this pull request Nov 4, 2024
HeikoKlare pushed a commit to HeikoKlare/www.eclipse.org-eclipse that referenced this pull request Nov 5, 2024
HeikoKlare pushed a commit to HeikoKlare/www.eclipse.org-eclipse that referenced this pull request Nov 5, 2024
HeikoKlare pushed a commit to HeikoKlare/www.eclipse.org-eclipse that referenced this pull request Nov 5, 2024
HeikoKlare pushed a commit to eclipse-platform/www.eclipse.org-eclipse that referenced this pull request Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

noteworthy Noteworthy feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add experimental preference to enable update on runtime behavior

5 participants