search

aminomancer / uc.css.js

A dark indigo CSS theme for Firefox and a large collection of privileged scripts to add new buttons, menus, behaviors and other opinionated features. The theme is similar to other userChrome stylesheets, but it's intended for use with an autoconfig loader like fx-autoconfig, since it uses JavaScript to implement its more functional features.
Other
337 stars 27 forks source link

Make the screenshot button behave like the original #7

Closed OliverParoczai closed 3 years ago

OliverParoczai commented 3 years ago

The original screenshot button toggled the screenshot interface when it was pressed, while this script disables the button while the screenshot interface is open.

I just commented out the code that disables the button, so that if someone wants to revert it, they have the option to do that.

Comparision recordings: The original button vs The reimplemented button

aminomancer commented 3 years ago

I think that's because page action buttons don't ordinarily have a disabled state. There wasn't any CSS for disabled page actions either. That's why the script has to load a stylesheet. It actually sets an attribute shooting, not disabled. So it would be consistent with the toolbar widget and reflect its state. However there's a bug in the screenshot extension where the observer notification is sent to every window. I tried to work around it but there's not much I can do without editing the screenshot extension.

However that only happens if you enter screenshot mode in one window and switch to another window without taking a screenshot or exiting screenshot mode, so I still think the button should be styled somehow when it's active. The toolbar widget also blocks clicks while it's active, but maybe this is excessive.

One thing that occurs to me is that the sidebar toolbar widget becomes "checked" instead of "disabled" when it's active. That's because it observes a checkbox iirc, but we could do something similar. Again there's no styles for checked page actions so we'd still need to load a stylesheet, but it makes more sense to me if the button is highlighted when it's active rather than grayed out and disabled. So I think I'll just do that instead. After all, if you're gonna skip the disable styling altogether, there isn't any need for the notification observer in the first place, it'll just be wasting CPU cycles. So if you don't want any active styling it'd probably be better to comment out the addObserver call

aminomancer commented 3 years ago

https://github.com/aminomancer/uc.css.js/commit/ce23422bd74db8aa9d026e42b211fb0f2bd8aed5

I figured a slightly dimmer highlight than the hover state would look good.

OliverParoczai commented 3 years ago

I tested the updated version today, looks really nice! It looks really similar to the original now with the new highlight