Freeseer / freeseer

Designed for capturing presentations at conferences. Pre-fill a list of talks to record, record them, and upload them to YouTube with our YouTube Uploader.
http://freeseer.readthedocs.org
GNU General Public License v3.0
216 stars 110 forks source link

ConfigTool: General Widget update #610

Closed farazs closed 9 years ago

farazs commented 9 years ago

Fix #539

Tasks:

farazs commented 9 years ago

Which layout is better? Should there be spacing between the regular preferences and the "Reset" section to indicate that it's not really a setting itself?

general tab 3 general tab 2 general tab 1

farazs commented 9 years ago

Also, should both of the reset/clear buttons create confirmation dialogs before completing their action?

farazs commented 9 years ago

So I added the functionality for "Clear talks", but the problem is that the configtool doesn't have access to the talk editor to tell it to reload the database. Right now I've made it so that every time the talk editor is launched from the record tool it calls select() on the presentationModel of the talk editor. Is this too much unnecessary work if there are no changes to the talks?

An alternative would be to add a "dirty" bool to the database, which the talk editor can check every time it's show() method gets called. If the dirty is true then it can call select() to reload the presentationModel, otherwise not?

farazs commented 9 years ago

I added the necessary calls to translate() for the new text, titles etc.

What's the protocol for adding new/different text in terms of translation? Should I just add the calls to translate as I have now? Should I also populate the entries in the English translation file? Or do I have to add blank entries in the other translation files as well?

farazs commented 9 years ago

mtomwing suggested that the clear talks button should not be in the ConfigTool since we'd like to keep those separate, and then there wouldn't be a need to let the talk editor know that the talks have been updated. @dideler and @zxiiro do you agree? In that case I could just remove the button.

zxiiro commented 9 years ago

Yes clearing talks is a function of the talk editor not configuration.

farazs commented 9 years ago

Okay, I removed the clear talks option. I also have two options for the Reset section layout. Right now it's set to the first one. Is there anything else that needs to be done?

general option 2 general option 1

farazs commented 9 years ago

Changed the button sizes a bit and the confirmation dialog string. Screenshots: general tab screenshot general tab dialog

farazs commented 9 years ago

@mtomwing Okay, changed everything like you asked. I also went and refactored the AVWidget init so that there aren't any unnecessary class variables there either.

EDIT: There seems to be an issue with the Travis build. Looking into it now.

mtomwing commented 9 years ago

For whatever reason my "help us translate" button doesn't seem to have any left/right padding screenshot 2014-10-05 17 27 37 .

mtomwing commented 9 years ago

Seems to be good now. screenshot 2014-10-05 18 07 57

farazs commented 9 years ago

Merge time?

dideler commented 9 years ago

Nice work @farazs.

I have some feedback on the reset section (may have feedback for the code as well, still need to look at it). We can make it a bit more user friendly.

To add more information before the prompt, I think changing "Reset Settings" to "Reset settings to defaults" will make users feel more comfortable pressing the button since they now have a better idea what it does (and they don't know that there is a prompt with more info).

For the prompt:

"Defaults" doesn't need to be capitalized in the window title, but if you disagree then you can leave it.

The description can be a bit more descriptive. E.g.

Your Freeseer settings will be restored to their original defaults. This will reset ..., ..., and ....

The actions can be more descriptive as well. [Reset] [Cancel] (with cancel being default)

farazs commented 9 years ago

I looked at a bunch of prompts from programs I use often ie. chrome, utorrent etc. and they all seem to follow the pattern of Title : App name Message : "Are you sure you want to X?", or "X will happen. Are you sure?"

I can make the title "Reset to defaults" or even "Freeseer", it shouldn't matter too much. I don't think anyone actually ever looks at it when a prompt comes up. For the message, I'm not sure if we want the user to be reading tons (or if they will even read it). Resetting settings to defaults is pretty self-explanatory imo. I could make the message something like "All your Freeseer settings will be restored to their defaults. Are you sure?". But I don't know if enumerating all the settings it will reset is helpful since we do say all. What do you think?

And I think the actions have to be from QMessageBox but I agree, it should be "Okay" and "Cancel" since that seems more appropriate for an action like this. (Nvm, just noticed there's a Reset option)

Edit: And yeah i'll change the button name to "Reset settings to defaults" so that it's more clear before the user presses it.

farazs commented 9 years ago

@dideler How about a title of "Freeseer", a message of "Your Freeseer settings will be restored to their original defaults." and "Reset" and "Cancel" buttons?

farazs commented 9 years ago

Something weird happens when I use Reset. But Yes, No, Okay and Cancel all work fine. Here's what happens: okay prompt reset prompt

mtomwing commented 9 years ago

Merged in 19f959dc9b72650f794b8ec037d8bc2ef373463c. I ended up going with the Reset/Cancel button combo.

mtomwing commented 9 years ago

Thanks for your contribution!

dideler commented 9 years ago

I looked at a bunch of prompts from programs I use often ie. chrome, utorrent etc. and they all seem to follow the pattern of Title : App name Message : "Are you sure you want to X?", or "X will happen. Are you sure?"

This doesn't seem to be the case when I check Chrome.

image

it should be "Okay" and "Cancel" since that seems more appropriate for an action like this.

A UX rule of thumb is to stay away from button labels like "Yes", "No", "OK". You should try to be specific (e.g. a call to action) yet keep it concise. Reading the button should remind the user what action will take place. See http://ux.stackexchange.com/questions/9946/should-i-use-yes-no-or-ok-cancel-on-my-message-box (or any search about button labels WRT to UX will bring up similar results).

But I don't know if enumerating all the settings it will reset is helpful since we do say all. What do you think?

I do think saying what will be reset is useful, but in this case I've changed my mind because I think there would be too many things to mention. If there were only a few settings or we could word it so that it's not a lot of text then the extra information would help, but currently we risk putting too much text which dilutes the message.

farazs commented 9 years ago

The dialogs I checked were things like opening all bookmarks and stuff like that. In my opinion, something like resetting all settings is a way bigger deal in Chrome than in Freeseer. So if a user really wants to reset all their settings in Chrome they should be informed about all the changes that will occur but I don't think the same should be true for Freeseer. Recovering from an unintentional reset in Chrome would take a lot longer than in Freeseer.

Yeah I agree that having "Reset" would be better than "Okay" if all else were equal. But the user expects dialogs to have both options on the right hand side, and when the Reset option is set, it goes all the way to the left hand size (as shown in the screenshots in my earlier post). Imo the downside of the unintuitive layout is greater than the upside of the better wording.

But it's upto you guys in the end and I think you ended up merging with "Reset" instead of "Okay" which is fine too.