-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Small usability enhancements. #82
Conversation
Can one of the admins verify this patch? |
@@ -743,19 +741,28 @@ void OBSBasicSettings::on_baseResolution_editTextChanged(const QString &text) | |||
void OBSBasicSettings::GeneralChanged() | |||
{ | |||
if (!loading) | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!loading) {
…
}
according to the style guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noted
Added a message box to tell the user he needs a scene before adding a source. |
Done, I think these few feature are helpful from a user standpoint. Open for review. |
@@ -33,5 +33,6 @@ class NameDialog : public QDialog { | |||
NameDialog(QWidget *parent); | |||
|
|||
static bool AskForName(QWidget *parent, const QString &title, | |||
const QString &text, std::string &str); | |||
const QString &text, std::string &str, | |||
const QString &placeHolder = QString("")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those lines are too long, make sure to stay within the limit (other lines might be too long as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is weird, it is ok on my side...
Also squash commits like Socapex@f25a3695704d3e6fdab732fa0d2291605c016a17 and Socapex@e39b800ec641f51bc2a762fd9080a4dc2c320ff1 |
Woops, I think I caused a conflict with my last commit. Do you want to resolve? |
int count = 1; | ||
|
||
for (int i = 0; i < ui->sources->count(); ++i) { | ||
if (ui->sources->item(i)->text().contains(QT_UTF8(placeHolderText))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is too long, I suggest replacing placeHolderText on line 1040
QString const placeHolderText{QT_UTF8(obs_source_getdisplayname(…))};
to avoid this issue
Don't ask user to save changes every item switch. Apply button greyed out when there are no changes.
NameDialog can have placeholder text. Localize scene number. Pre-select scene and source text.
There was no need to ask to save changes when you switch settings page. Also, to help guide users, the Apply button is greyed when there no changes. ++cognitive stuff.