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

feat: zoom buttons #1333

Merged
merged 32 commits into from
Jul 9, 2024
Merged

feat: zoom buttons #1333

merged 32 commits into from
Jul 9, 2024

Conversation

afonsojramos
Copy link
Member

@afonsojramos afonsojramos commented Jul 5, 2024

This is the second iteration of the zoom feature after #1318.

Closes #1312.

Dark Light
image image

Had to add a timeout to the listening to the resize event because it was very trigger-happy. Found a fix by doing debounced update calls. More info here

@adufr @setchy Let me know your thoughts.

@setchy setchy added the enhancement New feature or enhancement to existing functionality label Jul 5, 2024
@setchy setchy added this to the Release 5.10.0 milestone Jul 5, 2024
@setchy
Copy link
Member

setchy commented Jul 5, 2024

@afonsojramos - nice pivot. Seems much more user-friendly and predictable.

minor nit: could it be a little smaller (vertically). Otherwise, lgtm!

Base automatically changed from feat/zoom-slider to main July 6, 2024 03:41
Copy link
Contributor

@adufr adufr left a comment

Choose a reason for hiding this comment

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

Nice, thanks 😄

@afonsojramos
Copy link
Member Author

@setchy updated the image now. Looking way sleeker now!

@afonsojramos afonsojramos requested a review from setchy July 8, 2024 17:18
@setchy
Copy link
Member

setchy commented Jul 8, 2024

@setchy updated the image now. Looking way sleeker now!

Our zoom controls went on a diet! Nice!

@setchy
Copy link
Member

setchy commented Jul 8, 2024

I'm unable to run this branch locally. Getting the following error in DevConsole 🤔
Screenshot 2024-07-08 at 10 42 05 AM

I've tried multiple times a pnpm clean && pnpm i && pnpm watch but no dice.

Other branches are ok...

@afonsojramos
Copy link
Member Author

I'm unable to run this branch locally. Getting the following error in DevConsole 🤔 Screenshot 2024-07-08 at 10 42 05 AM

I've tried multiple times a pnpm clean && pnpm i && pnpm watch but no dice.

Other branches are ok...

Just ran on main and I am also getting that error 🤔

@afonsojramos afonsojramos mentioned this pull request Jul 8, 2024
@setchy
Copy link
Member

setchy commented Jul 8, 2024

Error no longer in the console, but still loading as a blank screen for me 🤷‍♂️
Screenshot 2024-07-08 at 1 22 01 PM

@setchy
Copy link
Member

setchy commented Jul 9, 2024

I haven't been able to re-launch the app with a saved zoomPercentage. Is this working on your end @afonsojramos?

steps to reproduce:

  • set my zoom level to 80%
  • confirmed that local storage has zoomPercentage: 80 persisted
  • restarted the app
  • app loads at 100%, shows in the Settings view as 100%
  • local storage still shows zoomPercentage: 80

@setchy
Copy link
Member

setchy commented Jul 9, 2024

I haven't been able to re-launch the app with a saved zoomPercentage. Is this working on your end @afonsojramos?

steps to reproduce:

* set my zoom level to 80%

* confirmed that local storage has `zoomPercentage: 80` persisted

* restarted the app

* app loads at 100%, shows in the Settings view as 100%

* local storage still shows `zoomPercentage: 80`

pushed a small update to load the existing settings.

Copy link
Member

@setchy setchy left a comment

Choose a reason for hiding this comment

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

Awesome work, @afonsojramos - lgtm!

@afonsojramos afonsojramos merged commit 88c8b24 into main Jul 9, 2024
9 checks passed
@afonsojramos afonsojramos deleted the feat/zoom-buttons branch July 9, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GUI scale setting
3 participants