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

Javascript reorganization and webpack build #51

Merged
merged 17 commits into from Nov 17, 2016
Merged

Conversation

jbeezley
Copy link

The basics of this work, like drawing a map, set_center, etc. I think the annotations might be broken, and I didn't test much anything else. I'm just putting this out there to give others a chance to look it over in case anyone wants to bikeshed about the eslint style or file organization. I don't particularly care, but it's best to settle on something early.

Another thing to consider is to do what ipyleaflet (and others) do, which is the run the javascript build in setup.py. For now, it is just a manual step (npm run build).

@@ -0,0 +1,15 @@
import './css/styles.css';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it's not useful now, but it could be later. We could split the css into multiple files per component or use something stylus. The import here just makes webpack load it and dump everything to /static/styles.css.

@kotfic
Copy link
Contributor

kotfic commented Nov 16, 2016

Most of the default functionality implemented in the notebooks/ folder looks good to me. I am however seeing the map displaying full screen in a band above the notebook (instead of fixed on the right hand side).

I am into putting the npm run build in the setup.py. For better or for worse we've hitched our wagon to setup.py; both the nbextension and serverextension are also automatically installed by setup.py.

@jbeezley
Copy link
Author

I'm not seeing the issue with the map displaying full screen. Could you host the notebook so I can debug it?

@jbeezley
Copy link
Author

I'm having trouble getting the annotations working. When following the first example notebook, I get in the console

utils.js:785 Could not open comm  --  Error: Class jupyter.widget not found in registry

but

$ jupyter nbextension list
Known nbextensions:
  config dir: /Users/jbeezley/.jupyter/nbconfig
    notebook section
      jupyter-js-widgets/extension disabled
  config dir: /Users/jbeezley/git/geonotebook2/env/bin/../etc/jupyter/nbconfig
    notebook section
      geonotebook/index  enabled
      - Validating: OK
      jupyter-js-widgets/extension  enabled
      - Validating: OK

Have I done something dumb?

@jbeezley
Copy link
Author

I see, it is disabled in ~/.jupyter/nbconfig. I have no idea how that got there.

@jbeezley jbeezley changed the title [WIP] Javascript reorganization and webpack build Javascript reorganization and webpack build Nov 16, 2016
@jbeezley
Copy link
Author

Okay, I think I fixed the remaining issues.

@kotfic
Copy link
Contributor

kotfic commented Nov 17, 2016

This looks GREAT to me! thanks @jbeezley

@jbeezley jbeezley merged commit 6c5a3d1 into master Nov 17, 2016
@jbeezley jbeezley deleted the javascript-build branch November 17, 2016 14:47
danlamanna added a commit that referenced this pull request Dec 12, 2016
OpenGeoscience/geojs#623 was resolved and #51
implicitly upgraded our version to incorporate the new interface. As a
result, trying to make a rectangle annotation threw a
"JSONRPCError (-32000): list index out of range".
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.

None yet

3 participants