Closed farazs closed 9 years ago
Hmm.. I noticed it's "Prepare to Record" with the "record" being capitalized. Should it be "Leave Record Mode" instead of non-caps? Or maybe "Leave record-mode"?
I feel like having the prepare to record button on the opposite side of the controls will likely be confusing if the auto record button is between them. I can see a user could get confused with this control layout.
On the otherhand I'm personally still not convinced the auto record mode should even be in the UI. It's a feature that will be launched by someone who knows they want to use it so I think leaving it as a menu item should be fine. With it there a user who doesn't need it will likely be confused about it and the auto record mode is a very special use case too.
Thoughts?
Hmm.. I could swap the locations of auto record and prepare to record.
But yeah maybe auto record isn't a core enough function to be on the main GUI. Could change "Options" to "Tools" and put it in there? Or put it in "File"?
I also think that ideally the "Play" button should play the last video recorded in the currently selected talk/presentation, not just the last video recorded in general. But I'm not sure if I'd get time to implement that. What do you guys think?
On the otherhand I'm personally still not convinced the auto record mode should even be in the UI. It's a feature that will be launched by someone who knows they want to use it so I think leaving it as a menu item should be fine.
I was thinking the same so I'm glad someone else mentioned it. It's a pretty cool feature, and I don't like making the work of a previous contributor less prominent, but for the sake of improving the project I think it's better as a menu item.
Having it under "Options" as "Enable auto-record" / "Disable auto-record" might work. Some text in the status bar saying, for example "Auto-record mode disabled" might also be helpful.
I noticed it's "Prepare to Record" with the "record" being capitalized. Should it be "Leave Record Mode" instead of non-caps? Or maybe "Leave record-mode"?
I like the last option best. Some other suggestions:
I also think that ideally the "Play" button should play the last video recorded in the currently selected talk/presentation, not just the last video recorded in general.
I think it's better the user open the video directory if they want to have more control. The feature as it is now is mainly to quickly check that your recording is fine, which it solves fine IMO.
Now that the "prepare" and "record" buttons are on the UI at the same time, the record options will be disabled until "prepare" is pressed (as they should be). What if we make it slightly more obvious that "prepare" has to be pressed by making the button stand out more?
For example
Note that the styling currently looks out of place next to the other buttons, so those may need to look somewhat similar but less prominent when standby mode is disabled.
I like the idea of making the button stand out more. What exactly is the right way of doing that? I guess a stylesheet could change some attributes but I'm not sure how well that would work.
I'm not so sure about the "Standby mode" idea. It sounds cooler, but I feel it's less clear to the user what the button does. "Prepare to Record" is pretty clear and doesn't require any interpretation. I think relying on a tooltip or hovertext for the user to understand what the button does isn't such a good idea.
What exactly is the right way of doing that? I guess a stylesheet could change some attributes but I'm not sure how well that would work.
Stylesheet is the way to go. What exactly are you not sure about?
If you want to use the stylesheet from the above screenshot as a starting point, here it is:
self.standbyButton.setStyleSheet(
"color: white;"
"background-color: #47a447;"
"border-style: outset;"
"border-width: 2px;"
"border-radius: 10px;"
"border-color: #398439;"
"font: bold 14px;"
"min-width: 6em;"
"padding: 6px;"
)
As for the button text, let's keep the original wording.
Trying out new styles to match the new (slightly altered) Record mode button. Gonna work and moving the auto record to a menu so we won't have to style that anyway. There are different pressed and hover styles as well.
Screenshots:
Thanks for the update. Looks good.
I also moved the auto-record feature to the options menu and gave it a ctrl+R shortcut. Here's the new recording screen:
Added a Leave button to the Auto Record screen. Also, made a stylesheet for the AutoRecordWidget itself that makes the background white, instead of having the Qlabels make their own background white, which would let the spaces between them show as a different color.
Is there a better way to connect the button to the auto_record method so that it always passes False? Right now there's a new method that calls auto_record(False).
Made it so that the shortcut for entering auto-record also works in auto-record (for leaving).
Trying out a new button style. Let me know if this is better.
I think I prefer the new style. The more visible button boundaries makes them look more like buttons.
+1
I'd say this is ready to be merged once squashed, unless anyone else would like to review or has comments.
Awesome! Well, squashed and ready.
Okay great. I'll wait until tomorrow to merge so others have a chance to respond.
Can you prepend "Fix #622" to your commit message summary line?
Sure, sounds good.
Added code to catch the OSError exception if it is thrown.
Okay I figured out that focus issue. Spacebar now pauses and unpauses recording. This should be good to merge.
Merged, thanks Faraz!
Fix #622
Implemented the layout discussed in the issue as well as some other changes listed below.
Screenshots: Windows
Linux