-
Notifications
You must be signed in to change notification settings - Fork 305
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
New manager layout #636
base: master
Are you sure you want to change the base?
New manager layout #636
Conversation
Overall, I'm a fan of the concept, and you've done a nice job implementing it, but I'll focus on the faults for now. Bulk options shouldn't be triggered by simply enabling/disabling individual styles. Export and check for updates shouldn't be buried in bulk actions. If we're not showing applies-to strings anymore, which is fine by me, we might wanna consider pulling favicons by default, bc they're more integral w/out strings. Either way, the icon shown for missing favicons is too pronounced to repeat that often. Probably better off making a real subtle PNG. They're also too big, and should be the standard 16px. The default only showing 3 was designed for strings, and there's no reason to restrict icons until they don't fit in the allotted space. Not sure we should go that dark on the header, or that subtle on its icons. Style name clickable area should be the entire height of entry, and there's really no need for superfluous edit icons. I prefer external links over "home" icons, which seem overly intricate to be shown repeatedly. If we're leaving icons small, I prefer the previous trashcans. Usercss icons in actions should go to the far left, so they don't leave gaps when they're often absent. It also bugs me that "checked" icons are larger than their empty circle counterparts, and I'm not sure I'm crazy about all the green. There's a few other icons I'd like to nitpick or try alternatives for, but those are the main issues at this stage. Drag & drop highlight when dragging is kinda pinkish, making it seem like you're doing something wrong. Also, I don't think its functionality will be immediately obvious. There's just a tooltip saying "change style injection order", but when you see the drag & drop cursor, the natural assumption is that you can change the order of the list manually, which of course it doesn't. Not sure if it's still a WIP, but drag & drop doesn't seem to work at all like I'd expect. If I have it A-Z and drag one style, the whole injection order changes from actual install order to A-Z. Same thing in reverse for Z-A, and I imagine any other current filter ordering. Injection order should only shift on styles with a lower injection number, which should shift down one spot accordingly. Orders seem to be changing drastically based on which filter you're currently sorting by when dragging. I think drag & drop should only work when sorting by injection order. Its functionality definitely needs to be more obvious, maybe a "don't show this message again" type of popup explaining that you need to filter by injection order to be able to change injection order, and a little explanation of what changing it accomplishes. I think the natural assumption is "higher has priority", when it's the opposite. I was kinda looking forward to see what you came up with for the new filter UI implementation, but they're the same awkward checkbox/select. NBD, but you'd mentioned alternatives previously, so I was kinda surprised. I'd prefer something a little slicker than |
I thought about it, but that's a lot of network activity. I thought about caching the icons in localStorage to reduce this issue, but I stopped short when I determined that I'd have to convert each image into a base64 encoded version and store it that way.
You're right. The icon size is set to be the same as all the other icons. There is actually a css variable (
Yeah, it's still a work in progress. I wanted some more contrast on the manage page, but I guess
Yeah, I think we could move the home & support external links to the far right; on the other side of the update icon, then we could leave the UserCSS icon where it is... When you say you prefer external links over home icons, do you mean change the icon? What about the support icon? It's also an external link.
Yeah, I'll work on adjusting the sizes. The green is set as a background color, so it would be easy to change. I think I might make it a css variable as well. What default color do you suggest?
Yeah, I thought about changing it but it is really complex with all the different css filters being used to filter the entry rows. And I've been so busy I haven't had time to think of an alternate solution.
I thought about making it open a modal with the additional favicons, but I really opted for the simplest solution. Using a detail container keeps the accessibility and doesn't look too awful (at least to me).
|
There's gotta be a reasonable way to do it. How does TM handle it? We can keep the ability to disable it, but if we're only showing icons, they shouldn't be the same placeholder repeated endlessly. They're also still 20px which is too big.
Where exactly?
Much better. Maybe a bit lighter, like
On the right track, but it still doesn't work like I'd expect. When filtering by order, styles should be shown in order, but they're not initially, so when you change one, you're still changing the whole list drastically. You'd have to import fresh to see it, because after the initial drastic shift it behaves as expected, but that should never happen. I'm not sure you should even be able to reverse order in that mode.
Header is looking better. Tooltips could use some more contrast now, and the import tooltip overlaps the dropdown.
Yes. A bunch of little houses look silly to me.
I don't even get why they're a thing. Home is: and help is: Seems redundant. If you like em, it's NBD. As far as them being external, there's no indication, which is whatever. Our "homepage" icon has always been the external, so it's not gonna confuse anyone.
Still buried.
Still not the case. A couple other things I noticed. What happened to entry hover bgs? Kinda important in table-like layouts. We've also lost toph's hyphen js, so strings w/out spaces don't break. The available space is pretty long and the text is quite a bit smaller, so I think no-wrap ellipsis would be fine. I'm a fan of symmetry, so I like them all being the same height anyway. A lot of the flex vertical centering issues are simplified by specifying heights when you know what they should always be. Current filter header is too low bc of font-size/positioning of caret. Tried to point them all out in the same screenshot: |
I've added a to-do list of things that have been done and still need to be addressed and/or discussed to the first post to keep everything in one place... |
Yes there needs to be a toggle, but it defaults to whatever pref is set. Default pref is non-usercss, which is what I see.
When I initially filter by order, it still doesn't sort them in chronological order. We can't reorder a list that isn't in order to begin with:
You went from too big, to too small. Favicons should be 16px. Globe icons shouldn't have pointer cursor either.
Just undock your browser. It's not great curently, but with a few tweaks it'd be alright.
Nope. This alignment really bugs me: In this case, it's the caret positioning and font-size. A lot of other flex alignment issues where they get off by a pixel or two usually happen when a parent element wraps elements of slightly different heights. Simplest solution is never align parent elements when avoidable. Specify the height (100% if its parent is specified) and align the child elements.
I think they can go, but I don't really care. Your call.
Unless there's some terrible performance impact which can't be accounted for, I think we should show them by default.
I have no issue with them after you fixed the size and color. Not to say you couldn't try something different if you have any bright ideas.
I just dislike the Minor details: Header icons could use a little space between them. Also kinda difficult to make out the arrows in import/export icons. Style name hover shouldn't be gray. Use If I didn't mention it, it LGTM. I agree with everything left on the to-do list. |
I didn't get to everything, but I did:
Stuff I didn't get to yet, but I plan on doing it/something:
|
Updated:
I left the support icon in place because I have a feeling that some users may not associate the home page with reporting issues... maybe we should link them to discord instead? I'm still not sure what to do with the checkbox + filter dropdown - I'm starting to lean toward making the filter have 3 options: not applied, then the two states. I don't think this should hold up merging this PR. |
E.g i18n-title=key;param (single param only)
Fix issue with StylusDeepDark
I spent some time updating this branch this weekend (it only took a year 😿). The last change I made was switching the filter toggle/selects for buttons that add GitHub-like text to the search input. It's not 100% working:
I pushed it out to the branch so I could get some feedback on the filter process; and maybe some help troubleshooting the above problems. And it's also nice to have it working properly again, so I can do bulk actions :P |
Even just trying to load this branch to test was a nightmare. Man, do I regret testing this branch in my regular profile. I guess the Chromium key is not set, which I didn't realize until it was too late. My dumb ass tried to set it after the fact. Long story short, I managed to bork Stylus completely in my profile. I couldn't even uninstall and load the master fresh. Every time I restarted my browser it disappeared. Ended up having to revert to an older backup of my portable browser, and swap a bunch of updated configurations and DBs. I like the general layout concept, as I always did, but what little testing I did get to when loading this branch from scratch, seems like it's seriously buggy. Importing a good local DB was about ten seconds of ridiculous CPU. Once imported, all import/export functionality doesn't seem to work at all. Export-all in the header exports nothing. Select-all-batch exports some randomly low size, like 10% of the actual DB size. I didn't test much beyond import/export. I wouldn't mind pitching in on this, particularly the responsive design aspect, which is practically non-existent. It needs to be compatible with the master, and import/export needs to work as expected before bothering with anything else IMO. |
This comment has been minimized.
This comment has been minimized.
@Mottie : I was curious to test. By the look/color of the buttons I would expect the behavior to switch back to the default. |
Yes, thanks! I spent a lot of time trying to get everything updated and rebased to the master branch, so a bunch of bugs slipped through. I've been really busy with work, so I only get around to working on this feature when I get free time & motivation. @narcolepticinsomniac please feel free to mess around in this branch all you want. And @Procyon-b please feel free to contribute how ever you wish! |
I can't promise I'll contribute code. I've put some of my things on the backburner for a long time, and I would like to get back to them while under lockdown. ;) |
00eadf6
to
7963a3a
Compare
This PR is still a work in progress! It's not ready to be merged.
TO DO:
Major
Injection order (still needs a lot of work)- [ ] Implement the actual injection order of the styles (only the UI is wired up right now).
- [ ] Update injection order values after importing & deleting.
- [ ] Add (i) (info icon), next to "#" in header, explaining how and why someone would want to manually sort the injection order.
Minor
Features
Missing functionality & stuff that needs work:
Included functionality:
As this is still a work-in-progress, please test and share any feedback.