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

Extracted Bootstrap #268

Merged
merged 11 commits into from
Jun 6, 2016
Merged

Conversation

eneufeld
Copy link
Member

  • Bootstrap templates are now optional. To use you have to run build-bootstrap.
  • extracted css to single files, currently must be copied by hand to dist
  • fixed bugs with treemaster detail

@edgarmueller sorry this is a bit of a mess

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.0%) to 87.626% when pulling 18cfee4 on eneufeld:feat_bootstrap into 7876ce7 on eclipsesource:master.

'ngRoute',
//'ngResource',
//'ui.ace',
'jsonforms-bootstrap'
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of copying the whole app.js file we also could just include this line and make sure it is loaded after app.js in index-bootstrap.html

@edgarmueller
Copy link
Contributor

This already looks great and I have commented on some minor issues, but I also have some questions, since I'm am not entirely sure whether everything is supposed to work already:

  • Does the horizontal layout work (without bootstrap, I saw the flex is being used at some point)?
  • Does master/detail work (without bootstrap), clicking user/enemies in the person example has no effect?
  • Would it make sense to extract the whole bootstrap part and release it as a standalone NPM package that can be used with the 'core', i.e. it would overwrite existing renderers when it has been included.

@eneufeld
Copy link
Member Author

Hi thank you for the review.

  • horizontal layout should work
  • master/detail should work -> you have to press the chevron
  • ideally I would like to have a jsonforms-bootstrap.js which would contain only the bootstrap renderers and bootstrap dependencies and which could be loaded after the jsonforms.js, but I don't know how to configure the build accordingly.

@edgarmueller
Copy link
Contributor

Regarding the last point: Yes, that's also what I'd like to have. About the build: nevermind, we leave that up to @sdirix 😁

@edgarmueller
Copy link
Contributor

edgarmueller commented May 25, 2016

Ah, I forgot to copy a CSS file, of course. You should be able to use the this plugin and update the webpack files accordingly.

// at the top 
var CopyWebpackPlugin = require('copy-webpack-plugin');

and in the plugins section add this entry:

new CopyWebpackPlugin([
    { from: 'src/jsonforms.css' }
]),

Likewise for the bootstrap webpack.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.0007%) to 87.616% when pulling 7ba13cb on eneufeld:feat_bootstrap into e67107e on eclipsesource:master.

@edgarmueller edgarmueller merged commit 106889b into eclipsesource:master Jun 6, 2016
@eneufeld eneufeld deleted the feat_bootstrap branch December 7, 2017 15:08
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