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

actions as icons in manager #1409

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

actions as icons in manager #1409

wants to merge 2 commits into from

Conversation

tophf
Copy link
Member

@tophf tophf commented Feb 19, 2022

I started to implement install-from-url of the old Stylish FF (#39) i.e. the remote CSS is used as is, even without a usercss comment, with an auto-update, but failed to squish it in the manager as a button so I've tried switching to just icons.

This PR shows two + buttons in the manager:

image image

And adds a U checkbox in the popup to toggle Usercss mode of new styles:

image image

...and with the iframes:

image image

@tophf
Copy link
Member Author

tophf commented Mar 2, 2022

@narcolepticinsomniac, ping! I reworked it several times to address my own doubts, so WDYT?

@narcolepticinsomniac
Copy link
Member

I thought you were on the right track converting to icons. I was kinda hoping, as you kept fiddling with it, you'd keep consolidating. 'Publish', and 'Backup', could certainly be downsized in the same fashion, eliminating multiple bulky details, and saving a lot of space with a couple rows of icons/buttons.

I don't see the harm in leaving the traditional button styling surrounding icons, if only to leave the option of split-buttons, without them looking odd. I'd rather see a thicker plus sign split button for 'write a new style', with 'traditional' and 'usercss' as submenu items, with the current mode checked.

I'd like to do the same in the popup, tbh. An icon would be simpler to work around for 'write a new style', and you approved of other icons from the last abandoned attempt, besides the power button, which I quite liked.

old redesign icon

Anyway, my preference was for more consolidation, which you seem to have backed off from with the latest commits. The current changes seem more like a half measure, so I'm kinda hoping you go back to what you were doing originally, and even expand further.

Regardless, there's nothing wrong with the minor consolidation you've got going here currently. If you decide it's good enough, I'm not gonna give you a hard time about it.

'Actions' icons are a little crammed together vertically. There's enough room for 'usercss' pref to be the checkbox, instead of multiple 'write new style' options. Closing order/history dialogs doesn't update the hash.

@tophf
Copy link
Member Author

tophf commented Mar 2, 2022

I'm just being cautious. I wasn't around when you created #517, but seeing it being stalled, I had my doubts about this change. Then I remembered the standard feature of many full-fledged apps to toggle the captions on the UI buttons (show/selective/hide), which is what inspired my recent change and I want to try to reuse it for all our buttons eventually, i.e. not in this PR.

Re + menu, I'll try that.

@narcolepticinsomniac
Copy link
Member

I wasn't around when you created #517

Yeah, that was the whole idea. I was gonna combine a bunch of PRs, and ram all the changes through when you weren't around to veto. There was a lot of good stuff in that PR, but I mucked up the variables/theming. I'm still not sure we wouldn't have been better off merging and fixing the ill-advised parts later. Eventually, I just got sick of the whole process and gave up on it. Turns out, bickering details with you might be what helped keep my interest. =)

@narcolepticinsomniac
Copy link
Member

Not positive whether it's this PR, or the master, but 'find styles' throws an error. Also, as seen in the screenshot, sometimes reddit's returning one result, other times, the normal amount. Could be the two issues are related, idk.

error

@tophf
Copy link
Member Author

tophf commented Mar 6, 2022

Fixed the error. As for the styles, I can't reproduce the problem, but it sounds like a network/server problem of the second index because now the results are shown as soon as an index is downloaded, then updated for the slower one.

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.

2 participants