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

replace images with inline svg icons #31

Open
mojoaxel opened this issue Jul 27, 2019 · 33 comments
Open

replace images with inline svg icons #31

mojoaxel opened this issue Jul 27, 2019 · 33 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mojoaxel
Copy link
Member

mojoaxel commented Jul 27, 2019

we should get rid of these dist/img/network and replace them e.g. with some inline (dataurl) svg images.

@r3code
Copy link
Contributor

r3code commented Aug 31, 2019

Some SVG can be taken form here https://iconsvg.xyz/

@mojoaxel
Copy link
Member Author

We need replacements for the following icons:

image image image image image image

image image image image image image image

I think these can be done in pure css:
image image

@mojoaxel mojoaxel added the help wanted Extra attention is needed label Aug 31, 2019
@mojoaxel mojoaxel self-assigned this Aug 31, 2019
@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

I think these can be done in pure css:
image image

Pure CSS Close (cross) Icon https://codepen.io/Bharek/pen/XJvVLj

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

We need replacements for the following icons:

image image image image image image

image image image image image image image

I think these can be done in pure css:
image image

Should we maintain colours of icons? I think it would be better to have single colour icons aka flat.

@mojoaxel
Copy link
Member Author

mojoaxel commented Sep 5, 2019

Should we maintain colours of icons? I think it would be better to have single colour icons aka flat.

👍 I also think we should get rid of the colors. If we use inline SVG maybe the user could change the color with custom css later.

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

It could look like this, I just created a branch in my fork https://github.com/r3code/vis-network/tree/edit-mode-svg-icons to test icons, a took some icons from iconsvg.xyz and some if them I drawn myself.
vis-nav-controls

@mojoaxel
Copy link
Member Author

mojoaxel commented Sep 5, 2019

It could look like this

Wow! This would be a perfect replacement!

For me it is important that we get rid of the single image-files. They should be included e.g. as data-urls inside in its own css.

r3code added a commit to r3code/vis-network that referenced this issue Sep 5, 2019
@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

It could look like this

Wow! This would be a perfect replacement!

For me it is important that we get rid of the single image-files. They should be included e.g. as data-urls inside in its own css.

Hmm. I can try to embed them as data:svg+xml into CSS

r3code added a commit to r3code/vis-network that referenced this issue Sep 5, 2019
@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

If I embed like this - users can not change the color of the SVG.

div.vis-network div.vis-navigation div.vis-button.vis-zoomExtends {
    background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' class='vis-icon vis-icon--zoom-extends' width='30' height='30' viewBox='0 0 24 24' fill='none' stroke='%23808080' stroke-width='2' stroke-linecap='square' stroke-linejoin='arcs'%3E%3Cpath d='M7.027 7.027l9.946 9.946m0-9.946l-9.946 9.946m6.792-10.43h3.64v3.64m-7.278-3.64h-3.64v3.64m7.278 7.278h3.64v-3.64m-7.278 3.64h-3.64v-3.64' stroke-width='1.6'/%3E%3Ccircle cx='12.102' cy='12.102' r='10'/%3E%3C/svg%3E");
    bottom:50px;
    right:15px;
}

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

What about stroke='currentColor'?

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

@Thomaash what do you mean?
if we use SVG as background-image we have no access to it's internals. We can only apply CSS colors when SVG is inlined into HTML.

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

What about stroke='currentColor'?

It can ba applied only to inlined . Css not applied to other documents, when SVG is data:image it treated like external doc.

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

You have hardcoded color in your SVG stroke='%23808080'. Just try to replace it with stroke='currentColor' then it should respect the color set by CSS.

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

You have hardcoded color in your SVG stroke='%23808080'. Just try to replace it with stroke='currentColor' then it should respect the color set by CSS.

It won't work. Take a look https://jsfiddle.net/v3ag0cs6/37/

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

The value currentColor uses the text color (color) not stroke, see updated: https://jsfiddle.net/thomaash/x67gLzpa/ .

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

The value currentColor uses the text color (color) not stroke, see updated: https://jsfiddle.net/thomaash/x67gLzpa/ .

But it's only for inline SVG. Here my updated example https://jsfiddle.net/q9Lh017r/1/
CSS is not applied to data-url encoded SVG.
The only way to allow users change the color is to inline SVG.

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

Oh, you're right.

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

@Thomaash I propose at first to replace PNG to SVG - it the fastest way. Next we can open another issue to "embed svg icons into JS".

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

We can do it in two stages.

@r3code
Copy link
Contributor

r3code commented Sep 5, 2019

OK. I'll do the first - PNG to SVG replacement. It almost ready

@Thomaash
Copy link
Member

Thomaash commented Sep 5, 2019

Although we may cause some problems to people using this. They now have their .png images and we'll start loading .svg images which they don't have. It will be better to do it all at once so we can just say “hey, you no longer need the images” instead of making them update them just to throw 'em away with the next release.

@r3code
Copy link
Contributor

r3code commented Sep 6, 2019

@Thomaash ok. So you propose to inline the SVG to the JS code?

@Thomaash
Copy link
Member

Thomaash commented Sep 6, 2019

Yeah, although I would like them more inlined in CSS but since SVG injected into HTML from JS can be colored using CSS (some people will definitely appreciate this) I'd go with inlining into JS.

@mojoaxel
Copy link
Member Author

mojoaxel commented Sep 8, 2019

A little of topic but here is a litte example ho to replace the images with simple unicode icons (without optimization):
https://stackoverflow.com/a/45307412/722162
image

r3code added a commit to r3code/vis-network that referenced this issue Sep 8, 2019
r3code added a commit to r3code/vis-network that referenced this issue Sep 8, 2019
r3code added a commit to r3code/vis-network that referenced this issue Sep 8, 2019
r3code added a commit to r3code/vis-network that referenced this issue Sep 8, 2019
@r3code
Copy link
Contributor

r3code commented Sep 8, 2019

@Thomaash I have finished the replacement r3code@0394ef6
@mojoaxel Hmm. So what is better?

@r3code
Copy link
Contributor

r3code commented Sep 9, 2019

A little of topic but here is a litte example ho to replace the images with simple unicode icons (without optimization):
https://stackoverflow.com/a/45307412/722162
image

I propose to use SVG icons by default and add an extra example how to replace navigation buttons.
For 90% of users SVG buttons is pretty enough IMO. Other can override CSS using the example #31 (comment) by @mojoaxel

@Thomaash
Copy link
Member

Thomaash commented Sep 9, 2019

We should not use UTF-8 characters as icons. We do not ship any font and therefore there is no guarantee that the characters rendered will look the way we want them to look. We could end up with quite different characters or even rectangles or question marks if someone's font doesn't have these characters (who knows what fonts are people using on their pages or in their browsers in case of default font face). Images give us much more consistent experience, reliability and also more freedom when it comes to design.

@Thomaash
Copy link
Member

Thomaash commented Sep 9, 2019

I have a few questions and objections to the draft:

  • Manipulation GUI:
    • Why are the icons in circles? If you want to keep the circles (I would remove them personally) make them align with the edges of the buttons their in (the way it was before).
    • Why are some of the icons in circles so close to the circle? Their almost touching it. It looks really weird when some are nicely spaced and some are crammed.
    • Why does edit edge use the same icon as add edge? Especially since edit node uses edit icon which makes more sense.
    • Since there is special add edge icon there should also be special add node icon not just a plus in a circle.
    • The close button should definitely be bigger than it used to be but this seems a bit too big.
  • Navigation GUI:
    • The fit icon is too crammed compared to the other more spaced icons.
    • Why is there green glow after click? I though we decided to ditch colors. I suggest upscaling the icon on click instead of adding second circle around it.
    • The rest looks good.

@r3code
Copy link
Contributor

r3code commented Sep 9, 2019

* Why are the icons in circles?

Looks familiar.

* Why are some of the icons in circles so close to the circle?

Maybe because I draw myself very fast just to show others.

* Why does edit edge use the same icon as add edge?

Mistake )

* Since there is special add edge icon there should also be special add node icon not just a plus in a circle.

Maybe.

* The close button should definitely be bigger than it used to be but this seems a bit too big

I'd suggest to place it top left on the place of the Edit button, so when a user presses the Edit button it turns to "Close" button. So the toolbar would become more friendly. It would have: Close, Add Node, Add Edge buttons by default.
For me its counter-intuitive to have all buttons in one style and have a Close button in another.

@Thomaash
Copy link
Member

Thomaash commented Sep 9, 2019

I like the idea of having Close        Add Node   Add Edge   … buttons.

@r3code
Copy link
Contributor

r3code commented Sep 9, 2019

* Why does edit edge use the same icon as add edge? Especially since edit node uses edit icon which makes more sense.

Fixed r3code@35c0f9b

@mojoaxel
Copy link
Member Author

@r3code Did you open a PR yet? If not feel free to open a Draft Pull-Request so we can have a look 😃

@r3code
Copy link
Contributor

r3code commented Oct 27, 2019

@mojoaxel I did it my fork here r3code@35c0f9b. But I have to merge with current origin before PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants