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

Passing codemirrorOptions as a configuration #649

Closed
wants to merge 1 commit into from
Closed

Passing codemirrorOptions as a configuration #649

wants to merge 1 commit into from

Conversation

4mgad
Copy link

@4mgad 4mgad commented Oct 20, 2017

No description provided.

@codecov-io
Copy link

codecov-io commented Oct 20, 2017

Codecov Report

Merging #649 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted Files Coverage Δ
scripts/schemas/config.js 82.6% <ø> (ø) ⬆️
scripts/make-webpack-config.js 97.05% <ø> (ø) ⬆️
loaders/styleguide-loader.js 100% <ø> (ø) ⬆️
src/rsg-components/Editor/Editor.js 85.71% <100%> (-1.79%) ⬇️
...omponents/ReactComponent/ReactComponentRenderer.js 100% <100%> (ø) ⬆️

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

Could you also update the docs and add some tests if possible?

@@ -39,6 +39,18 @@ module.exports = {
map: 'lodash/map',
},
},
codemirrorOptions: {
type: 'object',
default: {
Copy link
Member

Choose a reason for hiding this comment

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

This will be overwritten if codemirrorOptions isn’t undefined, we need to merge objects manually using process function.

@@ -15,7 +15,7 @@ const filterComponentsWithExample = require('./utils/filterComponentsWithExample
// Config options that should be passed to the client
const CLIENT_CONFIG_OPTIONS = [
'title',
'highlightTheme',
'codemirrorOptions',
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change, we should keep the old option until the next major release. See updateExample option in the config schema.

editorConfig would be a more generic name, and better match existing options like compilerConfig.

@4mgad
Copy link
Author

4mgad commented Oct 20, 2017

Hi Artem, unfortunately I can't spend more time on this issue for the time being.

@sapegin
Copy link
Member

sapegin commented Nov 13, 2017

Closing in favor of #662.

@sapegin sapegin closed this Nov 13, 2017
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