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.

Sign up for GitHub

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

Jump to bottom

Add hotkey to reset app scale #5386

Merged
merged 4 commits into from
May 30, 2024
Merged

Add hotkey to reset app scale #5386

merged 4 commits into from
May 30, 2024

Conversation

Trapiz
Copy link
Contributor

@Trapiz Trapiz commented May 21, 2024

Use the Ctrl+0 or Meta+0 shortcut to reset the app scale to 1

Scale.demo.webm

Feature Preview

implements

PR Checklist

  • My code adheres to AppFlowy's Conventions
  • I've listed at least one issue that this PR fixes in the description above.
  • I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
  • All existing tests are passing.

@Trapiz
Copy link
Contributor Author

Trapiz commented May 21, 2024

A question I do have is whether I should be writing tests for this. I couldn't find any tests that were checking the scaling up or down, so I didn't add a test for resetting the scale. I'm willing to write some tests for scaling and resetting said scale if you feel it's needed to write tests for this

@LucasXu0
Copy link
Collaborator

A question I do have is whether I should be writing tests for this. I couldn't find any tests that were checking the scaling up or down, so I didn't add a test for resetting the scale. I'm willing to write some tests for scaling and resetting said scale if you feel it's needed to write tests for this

It would be great if you could add both tests.

@LucasXu0 LucasXu0 added improvements improvements on an existing feature v0.5.9 labels May 21, 2024
@Trapiz Trapiz marked this pull request as draft May 21, 2024 11:48
@Trapiz
Copy link
Contributor Author

Trapiz commented May 21, 2024

A question I do have is whether I should be writing tests for this. I couldn't find any tests that were checking the scaling up or down, so I didn't add a test for resetting the scale. I'm willing to write some tests for scaling and resetting said scale if you feel it's needed to write tests for this

It would be great if you could add both tests.

Alright! I'll start looking into writing some tests. I converted the PR to draft for now.

@Xazin Xazin linked an issue May 21, 2024 that may be closed by this pull request
[FR] Hotkey to reset application scale to 1.0 #5340
Closed
@Trapiz Trapiz force-pushed the reset_app_scale_hotkey_5340 branch 3 times, most recently from f870adf to 85dd4dd Compare May 23, 2024 08:13
@LucasXu0
Copy link
Collaborator

Hi, @Trapiz. Do you need any help? I can add a sample for your reference.

@Trapiz
Copy link
Contributor Author

Trapiz commented May 27, 2024 •
edited

Hi, @Trapiz. Do you need any help? I can add a sample for your reference.

I have looked into writing integration tests for this, but ran into the issue that the IntegrationTestWidgetsFlutterBinding binding is registered, and we also need the ScaledWidgetsFlutterBinding binding to be registered to allow scaling. But this is currently not possible to have two bindings registered at the same time. Me and @Xazin already talked about this and he also couldn't find a solution to this problem. I am going to see if I can pull this off with unit tests but I don't think they are going to turn out to be good test cases.

But maybe you have a completely different approach/idea and I would love to see the sample!

@Trapiz
Copy link
Contributor Author

Trapiz commented May 29, 2024

Hi @LucasXu0, a quick follow-up to the comment above, I have looked into writing some unit tests but quickly realized that this would not really be possible. There are really no standalone components to test, and as far as I know, no possibility to trigger key presses either. I feel like with the way that integration tests are currently set up, it's not possible to test this with our current implementation. Do you have any ideas on how to proceed?

@LucasXu0
Copy link
Collaborator

LucasXu0 commented May 29, 2024 •
edited

@Trapiz Let me see if I can do that.

You're right. IntegrationTestWidgetsFlutterBuilder will conflict with ScaledWidgetsFlutterBinding. It seems there's no easy way to test the scale feature.

@LucasXu0 LucasXu0 marked this pull request as ready for review May 29, 2024 14:05
@Xazin
Copy link
Collaborator

Xazin commented May 29, 2024

It's the same with unit test, except it's one of two bindings AutomatedTestWidgetsFlutterBinding or LiveTestWidgetsFlutterBinding.

@Xazin Xazin merged commit 859eaf9 into AppFlowy-IO:main May 30, 2024
13 checks passed
zoli added a commit to zoli/AppFlowy that referenced this pull request Jun 2, 2024
@zoli
style: Merge branch 'main' into feat/time-field-type-2
fae5607 
* main: (25 commits)
  fix: sidebar issues (AppFlowy-IO#5444)
  fix: notification test (AppFlowy-IO#5440)
  fix: some list icons don't align with the paragraph (AppFlowy-IO#5439)
  feat: integrate show notification button option (AppFlowy-IO#5302)
  feat: add border to selected unsplash image (AppFlowy-IO#5428)
  feat: sidebar UI Revamp on mobile (AppFlowy-IO#5418)
  chore: update German translations with Fink 🐦 (AppFlowy-IO#5421)
  chore: update Spanish translations (AppFlowy-IO#5205)
  feat: video block support (AppFlowy-IO#5199)
  feat: add reset app scale hotkey (AppFlowy-IO#5386)
  fix: sidebar issues on Windows and Linux (AppFlowy-IO#5431)
  feat: support web layout setting and breadcrumbs (AppFlowy-IO#5425)
  fix: dragging the Unsplash cover triggers an assertion error (AppFlowy-IO#5404)
  chore: improve hover and text colors in dark mode (AppFlowy-IO#5416)
  fix: accept multi-key combination for customizing shortcuts & removes duplicates (AppFlowy-IO#5414)
  feat: sidebar UI Revamp on Desktop (AppFlowy-IO#5343)
  fix: default text direction not synced (AppFlowy-IO#5405)
  chore: update German translations (AppFlowy-IO#5382)
  feat: support preview grid/board/calendar block on web (AppFlowy-IO#5401)
  feat: support open row page (AppFlowy-IO#5400)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
improvements improvements on an existing feature v0.5.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FR] Hotkey to reset application scale to 1.0
3 participants
@Trapiz @LucasXu0 @Xazin