Skip to content

Scale to percentage #377

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

Open
wants to merge 6 commits into
base: beta
Choose a base branch
from

Conversation

hluedeke
Copy link
Collaborator

Pull Request Checklist

  • I have run yarn build to ensure that the project builds successfully.
  • I have updated CHANGELOG.md with the necessary changes made in this pull request.
  • I have added documentation for any new functionality from this pull request.

Description

This PR updates the scale menu to use percentages instead of decimal for scale. Percentage zoom is a much more familiar for those switching from Adobe.

  • Modifies the location of the scale message (please give feedback).
  • Modifies the "Open PDF" message slightly (adds styling)
  • Moves some button styling into the global css file, as this is a better method for global styling than using functions

Related Issues

#376

TODO

Review styling changes. You may disagree with them - would like feedback!

…percentage instead of a decimal scale. Styles the open pdf message a little bit more. Changes the styling of the scale message (see if you like it or not, it takes up a bit more screen real estate)
Copy link

vercel bot commented Mar 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
pattern-projector ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 22, 2025 5:27pm

@hluedeke
Copy link
Collaborator Author

I am going to close the PR temporarily after talking with Sasha. Will reopen if I get to a good place!

@hluedeke hluedeke closed this Mar 22, 2025
…ion over DM. Does not modify the input value to percentage, but instead uses the new proposed styling to add both units to the display bar
@hluedeke
Copy link
Collaborator Author

hluedeke commented Mar 22, 2025

I ended up removing the conversion to percentage in the actual scaling form, but since I changed the styling for showing scale, there is more room to show scale in both units (decimal scale and percent scale). This keeps the percentage zoom folks from asking questions but also (hopefully) prevents questions from those who are confused on decimal.

…percentage conversion in scale-menu. Was causing user input to not work correctly
@courtneypattison
Copy link
Collaborator

Sorry for taking so long to get back to you @hluedeke ! The whole family has been battling a nasty virus the last few days so I haven't had any time for PP.

I mostly agree with your changes! The scale factor is definitely confusing for folks. As @sashasewist mentioned, the reasoning for not using percentage is that it's too likely that people coming from Adobe will be confused and input their percentage. I like the switch to .05 step. I'm almost thinking maybe even lower to a .01 step since the people who have requested scaling all seem to be doing tiny scale changes. What do you think? Also, with the percentage now being shown in the banner, the 1% step may be easier to understand.

Thank you for all the styling cleanup! It's much needed.

The open PDF message is much prettier and easier to understand, thank you!

The size and subtlety of the scale banner scares me a bit. I've read about people ignoring the calibration warning and still cutting even though its right in the middle of the screen. I think it makes sense to make it a banner and also maybe format the magnify/zoom out messages in the same way but I think it needs to be obnoxious and in people's faces. I'm not sure how to make a sufficiently eye catching banner but I'd like to see one because my current styling looks bonkers, lol.

Thank you again! The code and front end always look cleaner after you send a PR!

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