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

UI: Upgrade stream key link to button in Wizard #2256

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

JohannMG
Copy link
Contributor

@JohannMG JohannMG commented Dec 18, 2019

Description

Follow up to previous pull request #2145
The wizard has a stream link URL as well. Adding the URL button in the
wizard to match. Additionally, fixing a few errors in the UI layout
and spacing where items were not padded.

Before:
Screen Shot 2019-12-17 at 1 07 52 PM

After:

Screen Shot 2019-12-17 at 2 44 13 PM

Motivation and Context

Matches UI of previous changes and fixes a button layout issue.

How Has This Been Tested?

Yes.
MacOS 10.14.6
MacBook Pro

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.

@Fenrirthviti
Copy link
Member

Could you also adjust the service selection text and box to line up left justified with the Please enter your stream information and Stream key text?

@JohannMG
Copy link
Contributor Author

JohannMG commented Dec 18, 2019 via email

@energizerfellow
Copy link

energizerfellow commented Dec 18, 2019

I don't like how the stream key field is abbreviated with the buttons on the same horizontal plane. The stream key field should be long enough to visually show the entirety of the stream key with the Show and Get Stream Key buttons below the stream key text field.

@JohannMG
Copy link
Contributor Author

JohannMG commented Dec 18, 2019 via email

@JohannMG
Copy link
Contributor Author

[ FYI I'll be out Dec 21 and more or less offline until 2020 ]

@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue Code Cleanup Non-breaking change which makes code smaller or more readable labels Dec 18, 2019
@JohannMG
Copy link
Contributor Author

JohannMG commented Jan 4, 2020

Any blockers here?

@jp9000
Copy link
Member

jp9000 commented Jan 4, 2020

I'm sorry, I thought you were coming back to address the issues Fenrir had pointed out.

The only thing I see is that you use QTStr with some URLs. I think you might have misunderstood and thought it was used to create a QString, but it's actually used to look up string translations, not to create a string. You shouldn't need to even create a QString directly, just assigning a normal C-string should automatically convert it to a QString. So you can just do qstr = "https://bla.com"; and that should be perfectly fine.

@jp9000
Copy link
Member

jp9000 commented Jan 4, 2020

Additionally I agree that the fact that the edit box is small is not that big of a deal. We can always just increase the size of the dialog or something to compensate later.

@JohannMG
Copy link
Contributor Author

JohannMG commented Jan 4, 2020

Okay.
Todo for this PR:

Definitely on the radar but not in this diff:

  • Fix various Wizard layout issues including like what Fenrir pointed out.

Will do possibly later

  • Change stream key box size.

Sound good?

@jp9000
Copy link
Member

jp9000 commented Jan 4, 2020

Sounds good.

@WizardCM WizardCM removed the Code Cleanup Non-breaking change which makes code smaller or more readable label Jan 5, 2020
@JohannMG JohannMG force-pushed the fix-wizard-key-link branch 2 times, most recently from 95f1553 to ee2c520 Compare January 7, 2020 20:35
@JohannMG
Copy link
Contributor Author

JohannMG commented Jan 7, 2020

Okay.

  • Updated to not use the QStr macro
  • Rebased onto last master

I hope everyone had a chill new years break.

Follow up to obsproject#2145: obsproject#2145
The wizard has a stream link URL as well, adding the button in the
wizard to match. Additionally, fixing a few  errors in the UI layout
and spacing where items were not padded.
@JohannMG
Copy link
Contributor Author

Updates were done: Good to merge?

@jp9000
Copy link
Member

jp9000 commented Jan 15, 2020

Yep. Thanks!

@jp9000 jp9000 merged commit 1bfe461 into obsproject:master Jan 15, 2020
@JohannMG JohannMG deleted the fix-wizard-key-link branch January 22, 2020 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants