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

Added support for Materialize CSS and Material Icons. #17

Merged
merged 6 commits into from
Mar 6, 2018
Merged

Added support for Materialize CSS and Material Icons. #17

merged 6 commits into from
Mar 6, 2018

Conversation

patrickberger
Copy link
Contributor

No description provided.

@schmunk42
Copy link
Collaborator

Replace EpicEditor with SimpleMDE

Is EpicEditor deprecated or why is it removed?

@patrickberger
Copy link
Contributor Author

The replacement of EpicEditor by SimpleMDE was done in master branch. Maybe @fenichelar could enlighten us.

@fenichelar
Copy link
Contributor

fenichelar commented Feb 28, 2018

@schmunk42 @patrickberger SimpleMDE includes a toolbar to make it easy for people who don't know markdown to write markdown, EpicEditor does not. I think it makes sense to have a toolbar because json-editor makes it easy for people who don't know JSON to write JSON. Thoughts?

image

@schmunk42
Copy link
Collaborator

I haven't looked to deep into the code, but ideally we can add editors via https://github.com/dmstr-forks/jdorn-json-editor#custom-editor-interfaces instead of having if statements in the code.

While we can deprecate features here and there, we should be able to maintain BC.

Would it be possible to remove EpicEditor and SimpleMDE from the core and add it via custom editor interfaces?

@fenichelar
Copy link
Contributor

It's been awhile since I looked at json-editor (over a year), but if I recall correctly, I made a simple swap from EpicEditor to SimpleMDE because SimpleMDE has a toolbar and there was a bug with EpicEditor when the data was undefined. See jdorn/json-editor#667.

It looks like this is a fork of a fork of a fork so I am not sure how my commit got here. I think that this PR should be limited in scope as much as possible.

@schmunk42 As far as removing EpicEditor and SimpleMDE, again, I think that should be separated from this PR as it is out of scope.

@patrickberger Can you remove my commits from this PR? It looks like they were already merged in this fork anyways.

@patrickberger
Copy link
Contributor Author

@fenichelar, @schmunk42 I removed my merge from master. Could you please check if this is ok? I am new to git and it took me some time and reading.

@patrickberger patrickberger reopened this Mar 1, 2018
schmunk42 pushed a commit that referenced this pull request Mar 6, 2018
* commit '27b611a766daf680f60f8b14779284bd9775175c': (44 commits)
  Added selenium tests with mocha
  Revert "Merge branch 'mehmetb-master' into develop"
  Include expanded schemas options to include refs (#27)
  Add option to have info buttons (#24)
  Implemented template support for 'rel' property on href's schema #606 (#17)
  Update multiple.js (#21)
  Fixed to expand refs before creating root editor (#18)
  fix for issue 627 - selectize dropdown causes prop to dissapear - and issue 538 - create hardcoded to true (#19)
  Issue 488 fix disabled fields get enabled when using properties dropdown (#22)
  Resolved merge conflicts
  update submitted json when editing select fields via json manually to (#29)
  Copy Array Element Button (#4)
  Cleaning with innerHTML='' causes issues on IE. So clean children one by one with removeChild (#11)
  use hasOwnProperty instead of requiring a "true" value for startval (#30)
  added check for whether label is defined for checkbox (#8)
  Respect "minimum"/"maximum" and "exclusiveMinimum/Maximum" (#7)
  Undefined variable issue when using template (#15)
  Fix typo (#31)
  Fix for crash given schema {"enum":[null]} (#26)
  add option for lodash templates (#5)
  ...
@schmunk42 schmunk42 merged commit b46fbee into json-editor:develop Mar 6, 2018
@schmunk42
Copy link
Collaborator

I've merged several PRs on develop - I think we have to remove the dist folder on development (master) branches and only include dist files for releases. Otherwise it's practially impossible to handle merges.

CC: @loganvolkers @xvaara @jdorn WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants