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

Fixes network bars position when a node is selected. #1667

Merged
merged 4 commits into from
Jul 13, 2016

Conversation

foot
Copy link
Contributor

@foot foot commented Jul 12, 2016

Bar position should be a function of node size!

Note: This is still an estimated position though it looks fine. Would be
nice to take into account size of labels (foreignObjects). The
transforms (for "undoing" the canvas zoom level) are a bit hectic here
and would be really nice to pull out and put in a wrapper...

Fixes #1663

Bar position should be a function of node size!

Note: This is still an estimated position though it looks fine. Would be
nice to take into account size of labels (foreignObjects). The
transforms (for "undoing" the canvas zoom level) are a bit hectic here
and would be really nice to pull out and put in a wrapper...)
@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

Not working for varied node sizes on the canvas (e.g. if you start w/ a very small browser window...).

Poking...

foot added 2 commits July 12, 2016 16:09
@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

Moves bar position

screen shot 2016-07-12 at 16 11 55

@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

Bar has moved above label as it was a bit tricky aligning it accurately in the middle of the text blob.

@rade
Copy link
Member

rade commented Jul 12, 2016

don't you end up with an awkward gap between node and text when not showing networks?

@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

No, you get a gentle "bump" when enabling/disabling the networks overlay.

I don't think its too jarring. Animating it could improve it further.

@rade
Copy link
Member

rade commented Jul 12, 2016

tried the branch. The "bump" is fine. animating it would be nice but not essential. HOWEVER, on firefox the bar overlaps with the text. and on Chrome it is very close to the text, much closer than on your screen shot above...

Firefox

Chrome

@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

Thanks for testing it. Testing out ff..

- Center it vertically properly.
@foot
Copy link
Contributor Author

foot commented Jul 12, 2016

Can do with another test! :)

@rade
Copy link
Member

rade commented Jul 12, 2016

LGTM. Though I cannot comment on the code.

@rade rade assigned paulbellamy and unassigned errordeveloper and 2opremio Jul 13, 2016
@paulbellamy
Copy link
Contributor

Code looks fine.

@foot foot merged commit 2219266 into master Jul 13, 2016
@rade rade deleted the 1663-fixes-network-bars-position branch July 5, 2017 13:11
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.

5 participants