Skip to content

Conversation

@danirabbit
Copy link
Member

@danirabbit danirabbit commented Nov 1, 2021

@cassidyjames cassidyjames self-requested a review November 8, 2021 19:47
Copy link
Contributor

@cassidyjames cassidyjames left a comment

Choose a reason for hiding this comment

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

Need to fix keyboard interactivity, I think I messed up resolving merge conflicts (and will try to fix that), and I have a question about the design:

Should we just go with the scale + spinbutton like we do in both Keyboard and Mouse & Touchpad settings? It seems like this is an appropriate place and would make it easier if you know a specific text scale works well on your machine versus having to just guess on a precise position based on the current design. I also think it would let us address my other inline comment about the start and end marks, since the scale would be even more self-evident.

I also think I'd actually like to try a less granular step; maybe snapping to every 0.125 instead of 0.05 because that feels really fiddly.

@danirabbit
Copy link
Member Author

Yeah good call about the spin button

@danirabbit
Copy link
Member Author

danirabbit commented Nov 10, 2021

@cassidyjames thanks for the thorough review here! I changed things to match the design of scale + spinbutton we have in other views like Mouse & Touchpad. So, removed the labels, notches on top. I also changed the set method from button release on the scale to a timeout on the adjustment. This fixes the keyboard case and should still have the same effect of keeping the UI from doing a stuttering resize while you're still dragging.

Also reverted the previous merge and fixed that, so you might have conflicts when you pull sorry!

Copy link
Contributor

@cassidyjames cassidyjames left a comment

Choose a reason for hiding this comment

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

Nice work, this seems a lot better.

@cassidyjames cassidyjames enabled auto-merge (squash) November 16, 2021 17:47
@cassidyjames cassidyjames merged commit 5fb9449 into master Nov 16, 2021
@cassidyjames cassidyjames deleted the text-sizescale branch November 16, 2021 17:48
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.

Text size presets are either too small or too large More granular text DPI scale

3 participants