Skip to content

Allow users to set frequency time #159

Closed
ghost wants to merge 1 commit intodevelopfrom
unknown repository
Closed

Allow users to set frequency time #159
ghost wants to merge 1 commit intodevelopfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 23, 2014

Preference list changed to edit text with validation. ( Integer.valueOf(str) > 0 && str.length() > 0 )

Preference list changed to edit text field.
Edit text field is generally idiot proof.
@eyedol
Copy link
Copy Markdown
Collaborator

eyedol commented Apr 25, 2014

From a first gland at the code, I'm not sure how we're calculating / indicating time as seconds as opposed to minutes. Say the user want the frequency time to be 30 seconds. Should it be float? say 0.1 or 0.30 to indicate seconds and then 1 or 30 indicates minutes. Thoughts?

I'm traveling so will be slow at reviewing but I give this a proper review once I settle in next week.

@ghost ghost changed the title Issue #101 - Allow users to set frequency time dont pull this Apr 28, 2014
@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 28, 2014

The issue mentioned only changing the form of input, not the type of input itself.
The time in the code is represented by integers indicating minutes, so it felt like it should stay that way.

I would keep the integer, but change its meaning from minutes to seconds. Then modify the content of the dialog something like "MM:SS". Maybe two edittext fields divided by colon, or one that displays a toast informing that time should like like mentioned above.

Waiting for your response.

@ghost ghost changed the title dont pull this Allow users to set frequency time Apr 28, 2014
@eyedol
Copy link
Copy Markdown
Collaborator

eyedol commented Apr 28, 2014

@123725 We can use something like a time picker? See this http://stackoverflow.com/questions/5533078/timepicker-in-preferencescreen

@eyedol
Copy link
Copy Markdown
Collaborator

eyedol commented May 8, 2014

@123725 thoughts on my previous comment

@KamilKalfas
Copy link
Copy Markdown
Contributor

@eyedol close this PR, i've created new one #171

@eyedol eyedol closed this May 30, 2014
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