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: Revision to streaming/recording indicator #575

Closed
wants to merge 1 commit into from

Conversation

cg2121
Copy link
Contributor

@cg2121 cg2121 commented Jul 20, 2016

This is a revision from my original idea for the indicators. Instead of the buttons changing colors, icons are now used.

@ONSBalder
Copy link

Can one of the admins verify this patch?

@reboot
Copy link
Contributor

reboot commented Jul 20, 2016

The image looks rather large compared to the text. I add 4px padding to the image on each side and that made it look much nicer. Also Qt automatically downscales the image to match the text size, but it never upscales the image. So it is probably good to make the source image larger like 64px or something, so Qt can downscale it for larger font sizes. If the font is larger then the image and it is not upscaled then it would be too small.

For color blind people it would probably be nice if the stopped icon had a different shape, like a square that is usually used for "Stop" on video/audio players.

@cg2121
Copy link
Contributor Author

cg2121 commented Jul 20, 2016

My intention was to make to icons look like a red LED light. With the icons being 16x16, this makes them a consistent size with other icons.

@cg2121
Copy link
Contributor Author

cg2121 commented Jul 21, 2016

I have now made the icons 64x64, so they can scale to whatever size they need to be.

@cg2121
Copy link
Contributor Author

cg2121 commented Jul 21, 2016

As for color blindness, I believe as long as the active and inactive icons are completely different colors, in this case red and black, I think we should be fine.

@kkartaltepe
Copy link
Collaborator

The icon committed isn't monochrome red so it should be fine even for color blind individuals. Feel free to give http://www.color-blindness.com/coblis-color-blindness-simulator/ a whirl.

@cg2121 cg2121 force-pushed the stream-record-indicator-2 branch 2 times, most recently from d448906 to ebd3ed0 Compare July 21, 2016 07:41
@reboot
Copy link
Contributor

reboot commented Jul 21, 2016

I added a 4px padding to the 16x16 icon, that is 25% of the original icon size. It seems you added a 4px padding to a 64x64 icon which is much less and almost not noticeable. A 25% padding makes a big difference.

@cg2121
Copy link
Contributor Author

cg2121 commented Jul 21, 2016

It does look better when I put the 25% padding on the icons.

@cg2121 cg2121 force-pushed the stream-record-indicator-2 branch 2 times, most recently from fbbc03a to 8cbd047 Compare July 21, 2016 15:27
@rikai
Copy link

rikai commented Jul 21, 2016

Fwiw, i actually preferred the idea of the previous commit, at least as i understood it, not having actually tested it in master. From what i understood, it changed the color of the whole button when recording, correct?

In the our use case for OBS for my job, the OBS machine is all the way across the room and would be being controlled via a hardware switcher in the long term. So having a strong visual indicator, like changing the color of the entire button color, which is easily visible from a distance, would actually have been preferred.

The icon solution is essentially what wirecast does, and has been a problem for us verifying our recording status at a glance from the other side of the room and seeing the previous commit was a nice bonus for us in our slow transition to OBS.

Not saying this approach is a bad one, just offering a bit of input from another perspective. 👍

@cg2121
Copy link
Contributor Author

cg2121 commented Aug 6, 2016

I have just noticed that the lines were greater than 80 characters. I have resolved this now.

This adds a stream/recording indicator that uses icons. A red icon for when active and a black one when inactive.
@jp9000
Copy link
Member

jp9000 commented Aug 22, 2016

As for this PR, I'm just not feeling this proposed change unfortunately. Everything we try here still makes me feel uncomfortable with it. It just doesn't look/feel right to me. Besides, your system tray addition sort of solves this to some extent anyway.

We can try to think of some alternatives later, but for now we should just shelve this idea for the time being.

@jp9000 jp9000 closed this Aug 22, 2016
@cg2121 cg2121 deleted the stream-record-indicator-2 branch October 18, 2016 03:12
joelvaneenwyk pushed a commit to joelvaneenwyk/obs-studio that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants