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

Repeated commands: Select which sizes to cycle between on repeated half actions #1434

Conversation

Eskils
Copy link
Contributor

@Eskils Eskils commented Aug 2, 2024

Aim

As people use a variety of screen sizes, it is desirable to cycle between different window sizes instead of / in addition to ½, ⅔ and ⅓.
Example use cases:

  • Working on a laptop that you occasionally connect to a larger external display, you might use the 1/3rd fraction on the laptop display and 1/4th fraction on the external display.
  • If you never use one of the sizes, say 1/3rd fraction, it should be possible to disable it

Changes

  • Selecting On repeated commands: “cycle between sizes for half actions” shows checkboxes with the possible sizes to enable and disable.
  • Changed text of menu item from “cycle between ½, ⅔ and ⅓ for half actions” to “cycle between sizes for half actions”.
  • Two new user defaults: cycleBetweenDivisions and cycleBetweenDivisionsIsChanged.
  • Animate toggling Todo-mode in Settings (Outside scope of task).

Changes to Settings View in Storyboard

These changes are mainly made to accommodate animation of toggling the cycle sizes view and the Todo-mode view.

  • The parenting NSStackViews have Detaches Hidden Views set to false.
  • The cycle sizes view and the Todo-mode view have a height-constraint set to 0.
    • This constraint is deactivated/activated when showing/hiding the view.
  • Vertical Clipping Resistance Priority is set to 999 in order to avoid constraint conflicts when height is set to 0 (view is hidden).

This attached video shows the changes in action:

RectangleCycleBetweenSizesDemo.mp4

Considerations

  • In the old implementation, the user default altThirdCycle was used to change the order of the second/third cycle. I can however not find any setting for it, nor any other references to it in the code apart from it being declared in Defaults.swift. Is it okay to ignore this property? Should it be removed, or should I adapt my implementation to take it into account?

  • I have added one string, and changed one string. What are the procedures on how these are localized?

    • For the changed string of the menu item, I see that it is translated to some languages. Is it better to keep the outdated translated version, or remove the translations and use the updated text in English?

I hope you will consider my proposal.
Please let me know if there is anything I should change or do differently.

@rxhanson
Copy link
Owner

rxhanson commented Aug 5, 2024

Thanks! I haven't had a chance to dig through this yet, but the concept makes sense to me.

  • altThirdCycle came from a request I had via email, and at first it seemed like something that more people would want but it turned out that no-one else ever asked for it. I think it is acceptable to leave it out for the sake of moving forward.

  • Localization: I'll take a look and get back to you on this, but if a new string isn't added to the main.strings automatically then you'll need to add it to the base language (US English), and I think it should be automatically set as a placeholder for all existing localizations.

@rxhanson
Copy link
Owner

rxhanson commented Aug 8, 2024

Alright, sorry for the delay. Excellent work! All of the code changes look good to me, and I tested this out and I like it. There's a few things to iron out.

  • Changing the order of the cycling is going to throw off muscle memory for a lot of users, and the cycle was 1/2, 2/3, 1/3 because that came from Spectacle. Can you think of a good way to still provide that previous order of cycling? I'll give this some thought, too.
  • I was thinking maybe change "cycle between sizes on half actions" to "cycle sizes on half actions".
  • Perhaps we can just drop the "Cycle between sizes" text field altogether? It seems straightforward that the checkboxes are associated with the repeated commands without this. If the text field was kept, it could be changed to "Cycle sizes".

Remove cycle sizes options text field
Refactor
Change CycleSizeDefault key to `selectedCycleSizes`
Change cycleSizesIsChanges key to `cycleSizesIsChanged`
@Eskils
Copy link
Contributor Author

Eskils commented Aug 8, 2024

No worries about the delay. Thank you for your feedback—I like your way of thinking.

  • I agree that it is important not to break people’s habits with this change. I believe the simplest solution would be to sort the cycle sizes to gradually go upwards and then wrap around to the smaller sizes. This would make the full order: 1/2, 2/3, 3/4, 1/4, 1/3, and the default order would be the same as Spectacle.
  • I agree with your proposed changes to the texts—it is more concise.

I have committed these changes, and also:

  • Changed “cycle between division” to “cycle size” in the code as I believe this makes more immediate sense
    • This includes changing the keys for the user defaults.

Please let me know what you think.

@rxhanson rxhanson merged commit aec69ed into rxhanson:main Aug 9, 2024
@rxhanson
Copy link
Owner

rxhanson commented Aug 9, 2024

Perfect!

@magnetikonline
Copy link
Contributor

What a great addition ❤️ - awesome work @Eskils 👍

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