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

Android: Fix fast forward button and text centered in buttons #1146

Merged
merged 4 commits into from Jun 27, 2017

Conversation

Projects
None yet
4 participants
@BlisterB
Member

BlisterB commented Apr 18, 2017

I've changed the character for fast forwarding by "»", the ancient one was too problematic.
I also fixed the text which wasn't centered all time on buttons (Android is a mess for those kinds of things)

Fixes #1124.

@carstene1ns carstene1ns added this to the 0.5.2 milestone Apr 18, 2017

@carstene1ns carstene1ns added the Android label Apr 18, 2017

@Ghabry

Why did you remove OnDraw from VirtualButtonRectangle? Now the speedup button renders like a circle. o.O My intention for that button being a rect was that it is a special one. But we can of course make it a circle if you think that is better but please remove Rect then.

@BlisterB

This comment has been minimized.

Show comment
Hide comment
@BlisterB

BlisterB Apr 30, 2017

Member

I agree with you Ghabry, a rectangle is a better shape for this kind of buttons.
This code was the first step of a refactoring, I should have mentioned it.

I just added "press and release" behavior, the next step is to put back the rectangle shape.

Member

BlisterB commented Apr 30, 2017

I agree with you Ghabry, a rectangle is a better shape for this kind of buttons.
This code was the first step of a refactoring, I should have mentioned it.

I just added "press and release" behavior, the next step is to put back the rectangle shape.

@BlisterB

This comment has been minimized.

Show comment
Hide comment
@BlisterB

BlisterB Apr 30, 2017

Member

The rectangle shape has returned :).

Member

BlisterB commented Apr 30, 2017

The rectangle shape has returned :).

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Apr 30, 2017

Member

I'm not really sure how to label that checkbox for locking the Fast Forward button.

Don't really think "Enable the press-and-release behavior for Fast Forwarding button" is obvious.

Does the Android compat toolkit provide a toggle button?
I think something like.

Fast Forward Button: Hold (o ) Toggle

is better interface design.

https://cdn-images-1.medium.com/max/800/0*Yj3LanbQyOH9cvfW.jpg

Member

Ghabry commented Apr 30, 2017

I'm not really sure how to label that checkbox for locking the Fast Forward button.

Don't really think "Enable the press-and-release behavior for Fast Forwarding button" is obvious.

Does the Android compat toolkit provide a toggle button?
I think something like.

Fast Forward Button: Hold (o ) Toggle

is better interface design.

https://cdn-images-1.medium.com/max/800/0*Yj3LanbQyOH9cvfW.jpg

@BlisterB

This comment has been minimized.

Show comment
Hide comment
@BlisterB

BlisterB May 1, 2017

Member

For what I understood, toggle buttons are for Apple design.
The Android guideline doesn't recommend such buttons for a 2 options choice (we have the checkbox for "on/off" choice). Toggle buttons are recommended when used in clusters:
https://material.io/guidelines/components/buttons.html#buttons-dropdown-buttons

But the sentence I used is probably not the best. Maybe something like is better:
Fast Forward Button:
[x] Toggle behavior

What do you think?

Member

BlisterB commented May 1, 2017

For what I understood, toggle buttons are for Apple design.
The Android guideline doesn't recommend such buttons for a 2 options choice (we have the checkbox for "on/off" choice). Toggle buttons are recommended when used in clusters:
https://material.io/guidelines/components/buttons.html#buttons-dropdown-buttons

But the sentence I used is probably not the best. Maybe something like is better:
Fast Forward Button:
[x] Toggle behavior

What do you think?

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 1, 2017

Member

The problm is that it's not clear that the opossite of toggle is hold.

With toggle button I didn't mean what Android calls togglebutton but what is in my ascii art.

Google calls this "Switch": https://material.io/guidelines/components/selection-controls.html#

Member

Ghabry commented May 1, 2017

The problm is that it's not clear that the opossite of toggle is hold.

With toggle button I didn't mean what Android calls togglebutton but what is in my ascii art.

Google calls this "Switch": https://material.io/guidelines/components/selection-controls.html#

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
Member

carstene1ns commented May 1, 2017

Use Spinner for speedup mode instead of Checkbox
Allow setting the multiplier value from the app through jni and store
it in the preferences.
@carstene1ns

This comment has been minimized.

Show comment
Hide comment
Member

carstene1ns commented Jun 4, 2017

layout

@carstene1ns carstene1ns requested a review from Ghabry Jun 4, 2017

@carstene1ns carstene1ns changed the title from [Android] Fix fast forwarding button and fix text centered in button to Android: Fix fast forward button and text centered in buttons Jun 4, 2017

@Ghabry

Ghabry approved these changes Jun 9, 2017

Works flawless on my outdated system

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jun 23, 2017

Member

The remaining question is here, should the slider and drop-down change size on bigger screens? Currently there is a gap.

Member

carstene1ns commented Jun 23, 2017

The remaining question is here, should the slider and drop-down change size on bigger screens? Currently there is a gap.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jun 24, 2017

Member

Imo the slider should match the other sliders and take the full space.

For the combobox I think the current width is fine but I guess it uses the same Ui group as the slider, so just expand both.

Member

Ghabry commented Jun 24, 2017

Imo the slider should match the other sliders and take the full space.

For the combobox I think the current width is fine but I guess it uses the same Ui group as the slider, so just expand both.

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jun 24, 2017

Member

We should really redesign the option screens then. They look really bad on tablets.

Member

carstene1ns commented Jun 24, 2017

We should really redesign the option screens then. They look really bad on tablets.

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jun 27, 2017

Member

Will redesign this later (#1207), ready to merge.

Member

carstene1ns commented Jun 27, 2017

Will redesign this later (#1207), ready to merge.

@Ghabry Ghabry merged commit 143a5c7 into EasyRPG:master Jun 27, 2017

6 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details

@fdelapena fdelapena added help wanted and removed help wanted labels Jan 7, 2018

Ghabry added a commit to libretro/easyrpg-libretro that referenced this pull request May 22, 2018

Merge pull request #1146 from BlisterB/master
Android: Fix fast forward button and text centered in buttons
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment