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

Seekbar preference cleanup #70

Closed
2 tasks done
smichel17 opened this issue Aug 1, 2016 · 3 comments
Closed
2 tasks done

Seekbar preference cleanup #70

smichel17 opened this issue Aug 1, 2016 · 3 comments

Comments

@smichel17
Copy link
Member

smichel17 commented Aug 1, 2016

  • Only show the preview when dragging a seekbar (not when pressing and holding down without dragging, since this can result in a buggy-feeling "flashing" experience.
  • Refactor seekbarprefs to re-use code.

Here's a bunch of thoughts about filtering. I couldn't decide where to draw lines so I'm just going to dump them all here.

  • Dragging works really well and is really useful.
    • As a user (dark theme), "I wish I could preview what app X looks while the filter is active."
    • As a designer, I suspect that a much simpler solution would serve the user just as well. I'm going to keep thinking about what that should be.
  • For longer taps, the preview flickers on/off
  • For longer taps or if you press and hold, the temp/intensity/dim used is the previous setting, not the new one.

Disclaimer: I didn't look at logs, so I'm hypothesizing.

I looked at this a little bit, and it seems like 'OnSeekBarChangeListener` isn't getting called until I lift my finger. If so, that's a bug in android's seekbar, not red moon.

  • I noticed the three seekbars each have their own class, but contain a lot of duplicate code. I wonder if there is a way to refactor...

It seems like the biggest barrier to refactoring would be the moon icons. You could still keep the three separate classes, but make them extend a new class that contains all the duplicate logic... but I also wonder if we could just remove the icons. The only unique things are:

  • The icons are always visible
    • That seems valuable.
  • Each value is shown separately
    • I'm not convinced this is valuable. Happy to hear arguments that prove me wrong, though.
  • Instead, we could have a single moon icon next to the filter name and also in the dropdown.
    • Then you also get the benefit of being able to compare different filters visually
@raatmarien
Copy link
Member

Thanks for the detailed thoughts!

  • For longer taps, the preview flickers on/off

Could you explain how/when the preview flickers? Because I can't replicate any flickering on my device.

  • For longer taps or if you press and hold, the temp/intensity/dim used is the previous setting, not the new one.

Disclaimer: I didn't look at logs, so I'm hypothesizing.

I looked at this a little bit, and it seems like 'OnSeekBarChangeListener` isn't getting called until I lift my finger. If so, that's a bug in android's seekbar, not red moon.

I noticed this too. Seek bars in Android indeed only update their value when they are dragged or released. However, this is actually a 'feature' and not a bug. The seek bar doesn't update, because the touch event hasn't been determined yet if you tap and hold still on the seek bar. If you slide down or up, the touch event is interpreted as an scroll and thus the seek bar shouldn't be updated. It only updates if the user slides horizontally or releases in the same place. I think it is possible to only show the preview if the touch event has been confirmed as a seek bar event (if the seek bar has been changed), do you think this would be an improvement?

  • I noticed the three seekbars each have their own class, but contain a lot of duplicate code. I wonder if there is a way to refactor...

Yeah, I think they should probably be refactored as all extending a single class, like SeekBarPreference, which could handle updating the values and showing the preview.

  • Each value is shown separately
    • I'm not convinced this is valuable. Happy to hear arguments that prove me wrong, though.

I think the separate moon icons can be useful to users, because they make it more clear what is being changed by modifying a seek bar.

  • Instead, we could have a single moon icon next to the filter name and also in the dropdown.
    • Then you also get the benefit of being able to compare different filters visually

I particularly like the idea of putting the moon icons in the profile spinner, to illustrate the colour of the profile, but I'm not sure how much work it would be to implement.

@smichel17
Copy link
Member Author

Could you explain how/when the preview flickers? Because I can't replicate any flickering on my device.

Maybe "flash" would have been a better word than "flicker"

On my Samsung Galaxy S4, the preview takes just under a quarter second to turn on when I tap and hold. So if I tap and hold for just over a quarter second, the filter is only on for a fraction of a second. It's a bug in the design, not the code.

I think it is possible to only show the preview if the touch event has been confirmed as a seek bar event (if the seek bar has been changed), do you think this would be an improvement?

I think so.

  • If it's a scroll event, it should behave exactly as it does: immediate on; immediate off.
  • If it's a tap, I can think of two options.
    1. Do not show the preview
    2. Show the preview (instant on upon lift, then fade off)

Option 1 is safe. Option 2 is better but has a number of possible pitfalls:

  • The 1/4 second delay might make it feel awkward
  • There might be weirdness if I adjust another slider during the fade-out

There's no way to know without trying it, though; you can decide whether it's worth the time to develop something you might just throw away.

I think the separate moon icons can be useful to users, because they make it more clear what is being changed by modifying a seek bar.

That's fair. I don't feel strongly about this anyway.

I particularly like the idea of putting the moon icons in the profile spinner, to illustrate the colour of the profile, but I'm not sure how much work it would be to implement.

Might be quite a bit; I think the hardest thing here is that you'd want them to stay at their true color regardless of whether the filter is active.

I also read recently that Android N may include a built-in red filter, which means that the use cases for Red Moon may be fewer (but still exist, I think) in the future, unfortunately (or fortunately, depending on how you look at it).

@smichel17
Copy link
Member Author

For reference, there is now a generic SeekBarPreference that will eventually replace the others (or at least, they will all inherit from it). It is not currently used anywhere.

https://github.com/raatmarien/red-moon/blob/master/app/src/main/java/com/jmstudios/redmoon/preference/SeekBarPreference.kt

@smichel17 smichel17 changed the title Filtering grab bag Seekbar preference cleanup Feb 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants