UI: Upgrade stream key link into button #2145

JohannMG · 2019-10-25T05:35:47Z

Description

Most of the top streaming services now have a link in the Stream Key label.
Also added Twitter/Periscope producer link.

Shows button when link is available:

Hides button when link is not available:

Motivation and Context

The link in the stream key label is unclear.

Upgrading the link to a button increases awareness of this assistant to the important streaming step more clear.

Open to suggestion on button text but I found this was the clearest, short phrase.

How Has This Been Tested?

Compiles and runs as expected on MacOS.
Button opens default browser as expected
Button shows only when there's a link available
Hover tooltip shows the link
Button link changes with services

Types of changes

New/upgraded feature (non-breaking change which adds functionality)

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 · 2019-10-25T05:39:04Z

You will need to squash your commits and update them to follow the appropriate commit guildines (prefix and present tense). Please make sure you read the contributing doc fully before marking this part of the PR template.

JohannMG · 2019-10-25T05:59:37Z

@Fenrirthviti Sure thing.
Mistakenly thought PRs squashed automatically: Updated.
Let me know if I missed anything else, I'm coming from a long bought of mercurial here.

RytoEX · 2019-10-25T06:14:52Z

This PR seems to include unrelated changes in .gitignore and plugins/enc-amf. Please fix this so that the PR only contains changes relevant to the PR's intent.

The commit message has some spelling and grammar mistakes. Please review the commit message and address these as needed.

Please also make sure that you end files with a newline (see UI/url-push-button.cpp and UI/url-push-button.hpp).

JohannMG · 2019-10-25T06:20:24Z

Will fix in the morning: And planning to add .gitignore to the message to keep it for other VSCode users unless there's a strong objection.

…

On Thu, Oct 24, 2019 at 23:16 RytoEX ***@***.***> wrote: This PR seems to include unrelated changes in .gitignore and plugins/enc-amf. Please fix this so that the PR only contains changes relevant to the PR's intent. The commit message has some spelling and grammar mistakes. Please review the commit message and address these as needed. Please also make sure that you end files with a newline (see UI/url-push-button.cpp and UI/url-push-button.hpp). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2145?email_source=notifications&email_token=AAMB7NFPGVKIXRS5LJEZI7LQQKFLRA5CNFSM4JE6YICKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECHJZKY#issuecomment-546217131>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMB7NEX6UKVJRK77WFHSO3QQKFLRANCNFSM4JE6YICA> .

Xaymar · 2019-10-25T06:37:21Z

And planning to add .gitignore to the message to keep it for other VSCode users unless there's a strong objection.

That change should likely be its own PR then, instead of being mixed into this PR.

JohannMG · 2019-10-25T19:58:56Z

Updated:

removed .gitignore change; will do in another diff
added newline at EOF
removed accidental AMF plugin update inclusion

JohannMG · 2019-10-25T20:09:23Z

Separate PR for .gitignore addition: #2147

JohannMG · 2019-10-25T20:19:07Z

@RytoEX So I did add the newline but the clang formatter removes it, then the CI rejects the files because of the newline addition like so:
(from https://dev.azure.com/obsjim/obsjim/_build/results?buildId=1929&view=logs )

=================================
Files were not formatted properly
UI/url-push-button.cpp
UI/url-push-button.hpp
=================================

I can't find a reference in contribution docs on proper EOF besides clang format which removes it. Is there a format check override that expected maybe? Otherwise I'll change this to not have newlines so the CI is appeased. Thanks

JohannMG · 2019-10-25T21:06:24Z

@RytoEX removed newline to appease CI * clang-format to keep this unblocked. I'll circle back if the CI fails again

Vainock · 2019-10-25T05:57:00Z

UI/data/locale/en-US.ini

@@ -162,6 +162,8 @@ Basic.AutoConfig.StreamPage.ConnectAccount="Connect Account (recommended)"
 Basic.AutoConfig.StreamPage.DisconnectAccount="Disconnect Account"
 Basic.AutoConfig.StreamPage.DisconnectAccount.Confirm.Title="Disconnect Account?"
 Basic.AutoConfig.StreamPage.DisconnectAccount.Confirm.Text="This change will apply immediately. Are you sure you want to disconnect your account?"
+Basic.AutoConfig.StreamPage.GetStreamKey="Get Stream Key"
+Basic.AutoConfig.StreamPage.GetStreamKey.ToolTip="Open in browser to fetch streamkey"


Maybe for consistency write it as "stream key".

That line is no longer used b/c I decided it made more sense it to have the hover tooltip show the URL destination. I will remove in a moment

dodgepong · 2019-10-30T15:23:09Z

On the whole I think this is a fine change. My only concern from a usability perspective is that the "Get Stream Key" button being on a different line from the actual Stream Key text box might be slightly more confusing to users, as I can see people not reading past the "Stream Key" box and wondering what they should do. Maybe it would be better if the button were moved to the right of the "Show" button, instead of on a different line?

jp9000

Looks good, just needs that change to the stacked widget page, and probably dodgepong's suggestion. The layout you'd want to insert it in to is probably the streamKeyWidget widget.

jp9000 · 2019-10-30T15:25:52Z

UI/forms/OBSBasicSettings.ui

@@ -119,7 +119,7 @@
     <item>
      <widget class="QStackedWidget" name="settingsPages">
       <property name="currentIndex">
-        <number>0</number>
+        <number>1</number>


This needs to be changed back to 0, otherwise the user will start on the "stream" page section when they enter settings. It's supposed to start on the "general" page normally. Don't worry, even I've done this before, it's just an issue with Qt designer more than anything. The page you last save it on becomes the default starting page.

(This applies for all stacked widgets by the way, so any stacked widget left on the wrong page has to be changed back to the original default. Just an annoying Qt designer issue.)

Fenrirthviti · 2019-10-30T15:35:54Z

Looks good to me as well.

Only point of feedback that can be addressed in a later update is that the URLs are currently hardcoded, which means if anything changes with those dashboard URLs on the stream service side we have no way of updating them without doing a full release. I would propose they would be right at home as a new field in services.json which we can already update remotely if needed. Not necessary for this PR, but wanted to make sure the thought was logged.

JohannMG · 2019-10-30T18:47:03Z

...Maybe it would be better if the button were moved to the right of the "Show" button, instead of on a different line?...

I have no problem with this for now.

This needs to be changed back to 0, otherwise the user will start on the "stream" page section when they enter settings...

Oops will amend.

Only point of feedback that can be addressed in a later update is that the URLs are currently hardcoded...

Agree. It was existing already but string matching always flags a code smell but decided it to be a change to make in a different PR.

// Update in a few

Most of the top streaming services now have a link in the stream key label. Upgrading this button to a button clarifies the assistance for the important step of setting up a stream. Creates a new type of button for URL opening simply which also automatically updates the tootip to the current URL. Includes addition of Twitter/Periscope URL to make this feature more complete.

JohannMG · 2019-10-30T19:10:31Z

Requested changed made.
New screenshot of button placement:

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 force-pushed the UpgradeStream-Link branch from 5125c60 to 2342242 Compare October 25, 2019 05:55

JohannMG force-pushed the UpgradeStream-Link branch from 2342242 to a57d6fa Compare October 25, 2019 19:56

JohannMG force-pushed the UpgradeStream-Link branch from a57d6fa to b62b9c2 Compare October 25, 2019 21:04

Vainock suggested changes Oct 26, 2019

View reviewed changes

RytoEX added the Enhancement Improvement to existing functionality label Oct 27, 2019

JohannMG force-pushed the UpgradeStream-Link branch from b62b9c2 to 41cceb6 Compare October 28, 2019 19:50

jp9000 reviewed Oct 30, 2019

View reviewed changes

JohannMG force-pushed the UpgradeStream-Link branch from 41cceb6 to 3f6cf0e Compare October 30, 2019 19:09

jp9000 merged commit e941eb6 into obsproject:master Nov 1, 2019

JohannMG deleted the UpgradeStream-Link branch November 26, 2019 23:43

JohannMG mentioned this pull request Dec 18, 2019

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

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Upgrade stream key link into button #2145

UI: Upgrade stream key link into button #2145

JohannMG commented Oct 25, 2019

Fenrirthviti commented Oct 25, 2019

JohannMG commented Oct 25, 2019

RytoEX commented Oct 25, 2019

JohannMG commented Oct 25, 2019 via email

Xaymar commented Oct 25, 2019

JohannMG commented Oct 25, 2019

JohannMG commented Oct 25, 2019

JohannMG commented Oct 25, 2019 •

edited

Loading

JohannMG commented Oct 25, 2019

Vainock Oct 25, 2019

JohannMG Oct 28, 2019

dodgepong commented Oct 30, 2019

jp9000 left a comment

jp9000 Oct 30, 2019 •

edited

Loading

Fenrirthviti commented Oct 30, 2019

JohannMG commented Oct 30, 2019

JohannMG commented Oct 30, 2019

UI: Upgrade stream key link into button #2145

UI: Upgrade stream key link into button #2145

Conversation

JohannMG commented Oct 25, 2019

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Fenrirthviti commented Oct 25, 2019

JohannMG commented Oct 25, 2019

RytoEX commented Oct 25, 2019

JohannMG commented Oct 25, 2019 via email

Xaymar commented Oct 25, 2019

JohannMG commented Oct 25, 2019

JohannMG commented Oct 25, 2019

JohannMG commented Oct 25, 2019 • edited Loading

JohannMG commented Oct 25, 2019

Vainock Oct 25, 2019

Choose a reason for hiding this comment

JohannMG Oct 28, 2019

Choose a reason for hiding this comment

dodgepong commented Oct 30, 2019

jp9000 left a comment

Choose a reason for hiding this comment

jp9000 Oct 30, 2019 • edited Loading

Choose a reason for hiding this comment

Fenrirthviti commented Oct 30, 2019

JohannMG commented Oct 30, 2019

JohannMG commented Oct 30, 2019

JohannMG commented Oct 25, 2019 •

edited

Loading

jp9000 Oct 30, 2019 •

edited

Loading