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

Add an indicator in the title bar if Streamer Mode is active #4410

Merged
merged 17 commits into from
May 27, 2023

Conversation

Mm2PL
Copy link
Collaborator

@Mm2PL Mm2PL commented Feb 24, 2023

Pull request checklist:

  • CHANGELOG.md was updated, if applicable
  • Move to title bar on windows
  • Add license for fluenticons
  • testing on windows would be appreciated

Description

Closes #4215.

Simply adds an icon into the Notebook bar or main window titlebar when streamer mode is enabled. Clicking said icon opens settings with Streamer Mode already in the search bar. Icon was made into dark/light variants and rendered into png by me :)

@Mm2PL Mm2PL marked this pull request as ready for review February 24, 2023 23:12
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/widgets/Notebook.cpp Show resolved Hide resolved
@Felanbird
Copy link
Collaborator

Felanbird commented Feb 24, 2023

Button is always there on startup no matter what option, toggling to enabled or disabled and then back to automatic toggles it as expected. to what it actually should be.

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Light mode on Windows (so just c-v-paste light mode handling from linux/macos portion) also missing, otherwise looks good and works as expected.
Would like to explore having a slight color on the icon, red/purple or something to bring more attention to it, but I would be happy to do that in a separate PR

src/Application.hpp Outdated Show resolved Hide resolved
src/widgets/Notebook.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/widgets/Notebook.cpp Show resolved Hide resolved
src/widgets/Window.cpp Show resolved Hide resolved
Copy link
Collaborator

@Felanbird Felanbird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality now works as I would expect it to 👍

just need the light-mode changes as pajlada stated

@pajlada pajlada changed the title Implement a Notebook button that shows when streamer mode is active Add an indicator in the title bar if Streamer Mode is active Feb 26, 2023
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small things, otherwise looks good & works as expected 👍

src/widgets/Notebook.cpp Show resolved Hide resolved
src/widgets/Notebook.hpp Outdated Show resolved Hide resolved
@Mm2PL Mm2PL requested a review from pajlada March 4, 2023 13:38
Copy link
Collaborator

@Felanbird Felanbird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
image

@8thony
Copy link
Contributor

8thony commented Mar 4, 2023

A small thing I noticed with a fresh install with no settings changed it only updates if you have a twitch channel added.

@Mm2PL
Copy link
Collaborator Author

Mm2PL commented Mar 4, 2023 via email

@Mm2PL Mm2PL added the Waiting for review PR bounced back to reviewer label May 17, 2023
@pajlada pajlada enabled auto-merge (squash) May 27, 2023 09:58
@pajlada pajlada merged commit c6c884d into master May 27, 2023
@pajlada pajlada deleted the feature/streamer_mode_indicator branch May 27, 2023 10:38
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -195,6 +195,11 @@ void SettingsDialog::filterElements(const QString &text)
}
}

void SettingsDialog::setElementFilter(const QString &query)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'setElementFilter' can be made const [readability-make-member-function-const]

Suggested change
void SettingsDialog::setElementFilter(const QString &query)
void SettingsDialog::setElementFilter(const QString &query) const

src/widgets/dialogs/SettingsDialog.hpp:60:

-     void setElementFilter(const QString &query);
+     void setElementFilter(const QString &query) const;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for review PR bounced back to reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants