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 help doc to timeshift tab explaining options #223

Closed
wants to merge 3 commits into from

Conversation

oneadvent
Copy link
Contributor

added file config_timeshift.html to accommodate this.

@linniksa
Copy link
Contributor

imho:

  1. "unlimited" should be movet upper "Max. *" option or inlines with that (because information is received from left to right and up to bottom)
  2. switch (checkbox) logic must be direct - in this case - [x] limit max period - enable text field...

@oneadvent
Copy link
Contributor Author

Hey @adamsutton do you think number 2 should be done? That changes the backend look at the checkbox so I didn't want to mess with it without your approval. Maybe a separate PR since this one really can't mess anything up and that potentially could.

@Partugal I agree with you on point 1 and this pull request now has that.

@adamsutton
Copy link
Contributor

@oneadvent I have merged you're first commit. I do not agree with changing the order (or inverting the checkbox), its simply not logical. The normal use case I would expect is that users would fill in the limits even if high (this is the safest approach). Also putting "Unlimited" before you've said what its for doesn't make sense. I agree @Partugal that putting it inline would be better (I wanted to do this with some other stuff on the EPG but could never figure it out).

I have removed the size stuff for now (it was never meant to be there) as it doesn't do anything just yet. And I've tidied the text up a bit.

@adamsutton adamsutton closed this Jan 13, 2013
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.

None yet

3 participants