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

FIX #42: Edge labels disappear when cluster is opened #43

Conversation

channeladam
Copy link
Collaborator

Fixes #42.

Automated tests updated to prove the solution.

@micahstubbs
Copy link
Member

thanks for the contribution @channeladam, taking a look at this now 😄

@micahstubbs
Copy link
Member

@channeladam looks like the Travis CI build failed. https://travis-ci.com/visjs-community/visjs-network/builds/97133771

could you run these commands to fix the build?

cd visjs-network

# install yarn js package manager, if not already installed
# https://yarnpkg.com/lang/en/docs/install/#alternatives-stable
curl -o- -L https://yarnpkg.com/install.sh | bash

# install js dependencies, if not already installed
yarn

# format code
yarn format

# add, commit, & push the result
git add .
git commit -m "yarn format"
git push

@micahstubbs micahstubbs self-requested a review January 14, 2019 06:59
@channeladam
Copy link
Collaborator Author

channeladam commented Jan 14, 2019

Thanks - sorry - I didn't know about the yarn format command...

Now the build is failing with gulp... Pretty sure I didn't cause that one! ? :)

Looks like yarn is trying to upgrade gulp from 3.9.1 to 4.0.0 and it is breaking...

6.99s$ yarn add gulp
yarn add v1.3.2
...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 61 new dependencies.
...
warning "gulp" is already in "devDependencies". Please remove existing entry first before adding it to "dependencies".
0.95s$ gulp
assert.js:42
throw new errors.AssertionError({
^
AssertionError [ERR_ASSERTION]: Task function must be specified
at Gulp.set [as _setTask] (/home/travis/build/visjs-community/visjs-network/node_modules/undertaker/lib/set-task.js:10:3)
at Gulp.task (/home/travis/build/visjs-community/visjs-network/node_modules/undertaker/lib/task.js:13:8)
at Object. (/home/travis/build/visjs-community/visjs-network/gulpfile.js:200:6)
at Module._compile (module.js:652:30)
at Object.Module._extensions..js (module.js:663:10)
at Module.load (module.js:565:32)
at tryModuleLoad (module.js:505:12)
at Function.Module._load (module.js:497:3)
at Module.require (module.js:596:17)
at require (internal/module.js:11:18)
The command "gulp" exited with 1.

@micahstubbs
Copy link
Member

micahstubbs commented Jan 21, 2019

ah! thanks for getting this far. will have a look.

@micahstubbs
Copy link
Member

screen shot 2019-01-20 at 10 12 08 pm

here's that gulp failure in the Travis logs https://travis-ci.com/visjs-community/visjs-network/builds/97236784

@micahstubbs
Copy link
Member

micahstubbs commented Jan 21, 2019

huh, the tests pass for me when I check out your branch locally 🤔

git checkout -b channeladam-fix-cluster-open-missing-edge-labels master
git pull https://github.com/channeladam/visjs-network.git fix-cluster-open-missing-edge-labels
yarn install
yarn test
# ...
  243 passing (3s)
  4 pending

✨  Done in 4.79s.

let's have a look and see why the tests fail in the Travis environment.

…ulp-version-for-travis

specify gulp@3.9.1 in travis config
@micahstubbs
Copy link
Member

micahstubbs commented Jan 21, 2019

@channeladam thanks for finding this bug in our travis config 😅
It should be fixed now with PR #49 that just went in.

when you have a moment, could you run these commands to apply the fix to your branch?

cd visjs-network
git checkout master
git remote add upstream git@github.com:visjs-community/visjs-network.git
git pull upstream master
git checkout fix-cluster-open-missing-edge-labels
git rebase master
git push

alternately, you could also just look at the diff here and add the new line to .travis.yml on your branch, then commit and push the result. https://github.com/visjs-community/visjs-network/pull/49/files

screen shot 2019-01-20 at 10 37 13 pm

@micahstubbs
Copy link
Member

fantastic, thanks for updating this PR @channeladam

let's get this fix in!

@micahstubbs micahstubbs merged commit a343aff into visjs-community:master Jan 27, 2019
Copy link
Member

@micahstubbs micahstubbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mojoaxel
Copy link

💌 Thanks @channeladam for your contribution!
This pull-request also has been merge into visjs/vis-network#15

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.

Edge labels disappear when cluster is opened
3 participants