-
Notifications
You must be signed in to change notification settings - Fork 293
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
SVG icons to replace htmlentities in right buttons #432
Comments
WIP - any comments? @DRSDavidSoft ? |
Thank you for creating these, they look great! 👍🏻 As a WIP they look amazing for the starting point! Personally, I only started working on the 🔎 icon, before work got busy, but the resulting design wasn't to my liking, so I haven't yet submitted it. I'm unfortunately still busy at work, for just a little more, but I'll be back soon to continue working on the open issues. In the meantime, I'd appreciate it if you could either attach the SVG files themselves, here or even better, create a new branch that would contains the assets, that way I could also submit PRs directly towards that branch. Once again, thank you, the attention to detail while re-designing the interface is certainly appreciated ❤️ |
Do you mind if I just stick these in master and let you revamp them later? I want to release a 6.0.1 soon since I've got quite a few minor bugfixes waiting for a tag. (Lazy object support will have to wait till 6.1 since it's a bit more complex than I thought) Side-note: Do you know if there's a simple way to import SVGs into sass? We're approaching double digits inlined svg files and it would probably be cleaner long-term to import them automatically, but I'd like to avoid a full-fledged JS build system |
I don't mind, but is there a particular reason not to create a branch specifically for this? I assume directly committing to master would be more convenient.
There are some small npm packages that are especially designed for this. I can work on a PR for it if you accept this route. Also, personally I wouldn't mind a full-fledged build system for the client side, if it'd be kept simple and fast. Also, thanks for attaching the SVGs, I'll try to make sure the new icons would be included in 6.0.1. Any upgrades and improvements can make it in 6.1! As always, thanks for all the hard work you do, each new release is always exciting! |
I was recently made aware of how bad it looks on other operating systems (Chiefly windows) so I really want a bear minimum released asap - we can make a branch to work on but a PR branch would be fine too
Sounds great. Should I just put these in for now and open a new issue for JS build tweaks or do you want to tidy them up first? |
If it's not too off-topic, can you please elaborate, are you referring to how bad the current icons look on Windows, or something else?
Let's commit the current icons and open an issue for the build system, I'll address both separately I'm a bit low on time so I'd like to apologize if any of the tasks take longer, I'll try to submit a simple solution and apply any improvements/enhancements later down the line Thank you |
Yeah exactly. I assumed if they had htmlentities they'd look good on all systems or at least on systems with the right fonts. I'll try to get these icons in but now I'm running into a new problem: When inserted as a background image you can't use CSS to set the colors and I don't want to bloat the output with a bunch of SVG markup. I'll probably end up using the new css mask property along with ::content |
@DRSDavidSoft The best laid plans... They look simple enough like that but needed to be much simpler to fit in the UI. The access path one was particularly illegible but I came up with a solution that's also much better since you can tell what it does just by looking at it. Chrome/FF at normal zoom on a normal DPI monitor: As you can see there's so little to work with I even had to get rid of the arrow in the folder... I'll upload the source files below. If you have no complaints I'll push this and tag it 6.0.1 |
Merged in 2fb169b and tagged 6.0.1 I'll leave this open for later discussion @DRSDavidSoft |
@DRSDavidSoft I could handle this if you're too busy, no rush anyway
The text was updated successfully, but these errors were encountered: