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

[UX] Gesture manager: add actions - toggle wifi and wifi on/off #4739

Merged
merged 1 commit into from Mar 5, 2019

Conversation

Projects
None yet
2 participants
@Frenzie
Copy link
Member

Frenzie commented Mar 5, 2019

As suggested by @poire-z.

#4727 (comment)

I wanted to make the gesture a W, but that's a bit too unwieldy. Instead there's the suggestion of a (backward) W.

g1048

@Frenzie Frenzie added the UX label Mar 5, 2019

@Frenzie Frenzie added this to the 2019.03 milestone Mar 5, 2019

@Frenzie Frenzie requested a review from poire-z Mar 5, 2019

NetworkMgr:turnOffWifi()

UIManager:show(InfoMessage:new{
text = _("Wifi disabled."),

This comment has been minimized.

@poire-z

poire-z Mar 5, 2019

Contributor

No timeout to this InfoMessage? If we use gestures, it's that we want things fast and get done :) No need for this additional tap to dismiss the InfoMessage I think.
I would even use a Notification with timeout, for even less disruption.

This comment has been minimized.

@Frenzie

Frenzie Mar 5, 2019

Author Member

Right, good point.

I was testing this and had no idea if turning it off worked or not without going to the menu, which seemed to defeat the point.

@poire-z
Copy link
Contributor

poire-z left a comment

Looks allright.
You don't want to add 2 others "Enable wifi" (unless all the code does it on demand) - and more important I think: "Disable wifi" - so a user can do that one just to be sure it's off, multiple times if he's the worrying type of person :)

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Mar 5, 2019

As a default gesture or just as available actions?

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Mar 5, 2019

As alternate available actions. Some users may prefer having two different gestures, some other only one that toggles I think.
No opinion about defaults or which of these should be set as default.

@@ -330,6 +339,9 @@ function ReaderGesture:buildMenu(ges, default)
{"toggle_gsensor", Device:canToggleGSensor()},
{"toggle_rotation", not self.is_docless, true},
{"toggle_reflow", not self.is_docless, true},
{"toggle_wifi", Device:hasWifiToggle() and "toggle_wifi", true},
{"wifi_on", Device:hasWifiToggle() and "wifi_on", true},
{"wifi_off", Device:hasWifiToggle() and "wifi_off", true},

This comment has been minimized.

@poire-z

poire-z Mar 5, 2019

Contributor

Looks strange. Shouldn't they be {string action_name, boolean enabled, boolean separator} ?
And I guess the separator could be a bit moved with the added stuff.

{"wifi_off", Device:hasWifiToggle() and "wifi_off", true},
{"toggle_wifi", Device:hasWifiToggle(), true},
{"wifi_on", Device:hasWifiToggle(), true},
{"wifi_off", Device:hasWifiToggle(), true},

This comment has been minimized.

@poire-z

poire-z Mar 5, 2019

Contributor

Still too many separators :)
All the 3 wifi ones should be in the same separated area.
And I guess you want to move toggle_reflow below them, near the Zoom to fit as it's kopt stuff

This comment has been minimized.

@Frenzie

Frenzie Mar 5, 2019

Author Member

Depends on whether you see it as the "toggle section" or the "zoom-type stuff section". I suppose "zoom stuff" makes more sense as a grouping.

[UX] Gesture manager: add action - toggle wifi and wifi on/off
As suggested by @poire-z.

#4727 (comment)

I wanted to make the gesture a `W`, but that's a bit too unwieldy. Instead there's the suggestion of a (backward) `W`.

The default gestures for wifi on and off are diagonal multiswipe half circles, like an turnable on/off knob.
@poire-z

poire-z approved these changes Mar 5, 2019

@Frenzie Frenzie force-pushed the Frenzie:gesman-toggle-wifi branch from b921a85 to ea55d4d Mar 5, 2019

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Mar 5, 2019

(No change, just had to manually merge it with the rotation toggle change.)

@Frenzie Frenzie merged commit 2f456df into koreader:master Mar 5, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Frenzie Frenzie deleted the Frenzie:gesman-toggle-wifi branch Mar 5, 2019

@Frenzie Frenzie changed the title [UX] Gesture manager: add action - toggle wifi [UX] Gesture manager: add actions - toggle wifi and wifi on/off Mar 5, 2019

@@ -118,6 +123,9 @@ local default_multiswipes = {
"northeast southeast",
-- "southwest northwest", -- visually ambiguous
-- "northwest southwest", -- visually ambiguous
"northwest southwest northwest",

This comment has been minimized.

@poire-z

poire-z Mar 5, 2019

Contributor

That one is as ambiguous as the previous one I didn't dare added :) So, we might as well dare.

This comment has been minimized.

@poire-z

poire-z Mar 5, 2019

Contributor

Mhhh, actually it is not :) it reads the same backwards following the arrows :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.