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

Add fullscreen functionality #653

Closed

Conversation

M4K14
Copy link
Contributor

@M4K14 M4K14 commented Sep 18, 2022

When you head to the Gesturefy settings>Extras and set these settings up:

wheel gesture = 'On'
wheel gesture mouse button = 'right'
wheel up = 'Toggle full screen'
wheel down = 'Toggle full screen'

Afterwards whenever you hold down the right click and scroll up, you repeatedly toggle the full screen up and down because you usually scroll the wheel rather than rolling it just as much as a single upward movement. Thus you usually get back to the screen size that you were already using and the window size is not properly toggled yet. that can be solved if we have a single "Full Screen" option which sets the window to full screen. then we can set the Gesturefy settings>Extras like this:

wheel gesture = 'On'
wheel gesture mouse button = 'right'
wheel up = 'full screen'
wheel down = 'Maximize window'

now you'll never face that problem ever again.

After
Before

Copy link
Owner

@Robbendebiene Robbendebiene left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request.
I'd prefer some changes in order to stay consistent with the other commands and generel conventions.

Comment on lines +736 to +743
const window = await browser.windows.get(sender.tab.windowId);

await browser.windows.update(sender.tab.windowId, {
state: 'fullscreen'
});
// confirm success
return true;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please have a look at the MaximizeWindow command implementation.
If the window is already in fullscreen mode the function should not return true.

},
{
"command": "Fullscreen",
"group": "window"
Copy link
Owner

Choose a reason for hiding this comment

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

The command is better suited for the window.controls group.

@@ -493,6 +493,10 @@
"message": "New private window",
"description": "New private window"
},
"commandLabelFullscreen": {
Copy link
Owner

Choose a reason for hiding this comment

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

Please rename the command and all it's occurrences from Fullscreen to EnterFullscreen or "Enter full screen" respectively.

@@ -723,7 +723,7 @@ export async function ToggleWindowSize (sender, data) {
// maximizes the window if it is already in full screen mode
export async function ToggleFullscreen (sender, data) {
const window = await browser.windows.get(sender.tab.windowId);

Copy link
Owner

Choose a reason for hiding this comment

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

Please remove unnecessary indents.

Comment on lines +850 to +851
"message": "Sets the window to its full size.",
"description": "Sets the window to its full size."
Copy link
Owner

Choose a reason for hiding this comment

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

Please change the description to: "Activates full screen mode for the current window." The main reason for this change is that "full size" might be misunderstood as "maximize the window"

@M4K14 M4K14 closed this Feb 16, 2024
@M4K14
Copy link
Contributor Author

M4K14 commented Feb 16, 2024

added another pull request with fixes and updates. thanks for the instructions.

@M4K14 M4K14 deleted the Add-Fullscreen-functionality branch February 16, 2024 05:40
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.

2 participants