Skip to content

Conversation

@gratcliff
Copy link
Contributor

@gratcliff gratcliff commented Sep 2, 2020

🧰 What's being changed?

  • Adding react-codemirror2 as core dep.
  • Refactoring directory structure for common usage
  • Using esm module syntax throughout
  • New Core Export via options -> Full CodeMirror Instance! Usage through { editable: true }
  • Codemirror runmode highlighting + style + line numbers via { highlightMode: true }

🧪 Testing

Can run locally via npm run start to see our full react-codemirror2 instance in action.

Gabe Ratcliff added 2 commits September 1, 2020 16:22
- Adding codeEditor structure from ui lib
- Restructuring src to be more comprehensive
- Added sass support in webpack
- Consolidating utils for shared use
- Porting ui code editor
@gratcliff gratcliff changed the title Feat/codeeditor export feat: code editor export Sep 2, 2020
Gabe Ratcliff and others added 16 commits September 1, 2020 18:18
- Tweaking linters for new file structure
- Test updates
- Prettyfying
* feat: attempt at line highlighting w comments

* feat: it works and is less ugly now

* fix: blocking linenumbers from selection

* chore: small eslint fix to pass tests

* feat(highlighting): minor refactor
- Updated name schemes
- Reuse components

* feat(highlighting): further refactoring
- Moving standalone function calls outside parent
- Consolidating component declaration

* fix(highlighting): applying overlay to deadspace

* chore(coremirror): remove extra class

* chore: adding variable

Co-authored-by: Gabriel Ratcliff <gaberatcliff@gmail.com>
Co-authored-by: Tony Li <runnabro@users.noreply.github.com>
@gratcliff
Copy link
Contributor Author

@rafegoldberg Would love your support here (or in a separate branch/PR) teasing out duplicate styles.

@gratcliff gratcliff marked this pull request as ready for review September 14, 2020 20:27
@gratcliff
Copy link
Contributor Author

gratcliff commented Sep 14, 2020

Just realized that I need to work on modifying the readme.md to this! Working on it now!

- Updating readme.md to provide proper use cases and option documentation
- Cleanup per review comments
Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

lgtm! feel free to merge when tests are all good. lmk and i'll tag a major version bump for you

@gratcliff
Copy link
Contributor Author

lgtm! feel free to merge when tests are all good. lmk and i'll tag a major version bump for you

Thanks a lot @erunion! I really appreciate the review. Let me get some more eyes on this before we go for a release!

@erunion
Copy link
Member

erunion commented Sep 14, 2020

When we go for a new release we should also remove this library from the api-explorer repo too since it's still there. I dinked around a bit with that two weeks ago but ran into a bunch of Jest compilation issues that are probably no longer a problem with the work here.

@gratcliff
Copy link
Contributor Author

When we go for a new release we should also remove this library from the api-explorer repo too since it's still there. I dinked around a bit with that two weeks ago but ran into a bunch of Jest compilation issues that are probably no longer a problem with the work here.

Sounds perfect! Next steps for this are integrating in the api-explorer and the ui libs.

- Excluding React and react-dom from webpack built as to avoid conflicts
- Replicating import syntax in test bed
- Cleaning up package.json
Copy link
Contributor

@rafegoldberg rafegoldberg left a comment

Choose a reason for hiding this comment

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

Just some minor clean up stuff and a couple of questions. Otherwise I'm in to this!

expect(code.hasClass('cm-s-neo')).toBe(true);
expect(code.html()).toBe(
'<span class="cm-s-neo"><span class="cm-keyword">var</span> <span class="cm-def">a</span> <span class="cm-operator">=</span> <span class="cm-number">1</span>;</span>'
'<div class="cm-s-neo"><span class="cm-keyword">var</span> <span class="cm-def">a</span> <span class="cm-operator">=</span> <span class="cm-number">1</span>;</div>'
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to refactor these HTML expectations in to snapshots at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, or vice versa!

Comment on lines +97 to +120
function flush() {
accum = opts.tokenizeVariables ? tokenizeVariable(accum) : accum;
if (curStyle) {
output.push(
// eslint-disable-next-line no-plusplus
<span key={++key} className={`${curStyle.replace(/(^|\s+)/g, '$1cm-')}`}>
{accum}
</span>
);
} else {
output.push(accum);
}
}

CodeMirror.runMode(code, mode, (text, style) => {
if (style !== curStyle) {
flush();
curStyle = style;
accum = text;
} else {
accum += text;
}
});
flush();
Copy link
Contributor

Choose a reason for hiding this comment

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

There are three different potential return values, right? Might be nice to break this up in to multiple methods, or add some comments above each section just so we don't forget down the line!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should only have 2 variant return values:

  • legacy
  • More traditional JSX/ DOM structure (divs, styling, etc).

They both rely on the same core to a large extent - but I broke out the styled part into it's own functional component!

Comment on lines +22 to +26
if (opts.highlightMode) {
classes = 'CodeEditor cm-s-material-palenight';
} else {
classes = opts.dark ? 'cm-s-tomorrow-night' : 'cm-s-neo';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should make a decision between these two dark themes regardless of rendering strategy. (I would personally vote for Material Palenight!) Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should, I think this should be bundled into a large decision/work to consolidate the css between this, the ui, and readme libs.

@gratcliff gratcliff merged commit 3c2baa0 into master Sep 15, 2020
@gratcliff gratcliff deleted the feat/codeeditor-export branch September 15, 2020 21:55
kellyjosephprice pushed a commit to readmeio/markdown that referenced this pull request Nov 9, 2020
### 🧰  Changes

Been a while since the syntax highlighter has been updated here and since I added support for TOML to it today, we should get it up to date here so Markdown code blocks with that language should get highlighted.

Includes a fix for how codemirror renders syntax highlighting as `div`s.

### 📖  Release Notes
#### 10.2.0 (2020-10-29)

* feat: adding support for TOML highlighting (#26) ([b2e617b](readmeio/syntax-highlighter@b2e617b)), closes [#26](readmeio/syntax-highlighter#26)

#### <small>10.1.2 (2020-10-26)</small>

* fix: isolating regex check to highlightmode option (#25) ([aec7c81](readmeio/syntax-highlighter@aec7c81)), closes [#25](readmeio/syntax-highlighter#25)

#### <small>10.1.1 (2020-10-23)</small>

* fix: check for \n in CodeMirror runmode (#23) ([66444f4](readmeio/syntax-highlighter@66444f4)), closes [#23](readmeio/syntax-highlighter#23)

#### 10.1.0 (2020-10-22)

* chore: moving react and react-dom into being peerdeps (#24) ([d1826d7](readmeio/syntax-highlighter@d1826d7)), closes [#24](readmeio/syntax-highlighter#24)

#### <small>10.0.1 (2020-10-12)</small>

* feat(context): exporting context provider (#22) ([1dc67c0](readmeio/syntax-highlighter@1dc67c0)), closes [#22](readmeio/syntax-highlighter#22)
* fix(readonly): wip styled output (#21) ([82b99ab](readmeio/syntax-highlighter@82b99ab)), closes [#21](readmeio/syntax-highlighter#21)

#### 10.0.0 (2020-10-05)

* fix(bundle): client and server compatibility (#20) ([bd4362b](readmeio/syntax-highlighter@bd4362b)), closes [#20](readmeio/syntax-highlighter#20)
* chore(deps-dev): bump @commitlint/cli from 9.1.2 to 11.0.0 (#10) ([c466e8d](readmeio/syntax-highlighter@c466e8d)), closes [#10](readmeio/syntax-highlighter#10)
* chore(deps-dev): bump @commitlint/config-conventional (#19) ([b700798](readmeio/syntax-highlighter@b700798)), closes [#19](readmeio/syntax-highlighter#19)
* chore(deps-dev): bump css-loader from 3.6.0 to 4.3.0 (#16) ([e3c0c67](readmeio/syntax-highlighter@e3c0c67)), closes [#16](readmeio/syntax-highlighter#16)
* chore(deps-dev): bump eslint from 7.8.0 to 7.10.0 (#11) ([d5e30e0](readmeio/syntax-highlighter@d5e30e0)), closes [#11](readmeio/syntax-highlighter#11)
* chore(deps-dev): bump husky from 4.2.5 to 4.3.0 (#18) ([7e437fe](readmeio/syntax-highlighter@7e437fe)), closes [#18](readmeio/syntax-highlighter#18)
* chore(deps-dev): bump prettier from 2.1.1 to 2.1.2 (#14) ([f3c572f](readmeio/syntax-highlighter@f3c572f)), closes [#14](readmeio/syntax-highlighter#14)
* chore(deps-dev): bump sass-loader from 10.0.1 to 10.0.2 (#12) ([2e9a2e9](readmeio/syntax-highlighter@2e9a2e9)), closes [#12](readmeio/syntax-highlighter#12)
* chore(deps-dev): bump terser-webpack-plugin from 4.1.0 to 4.2.2 (#15) ([9c0d3dc](readmeio/syntax-highlighter@9c0d3dc)), closes [#15](readmeio/syntax-highlighter#15)
* chore(deps-dev): bump webpack from 4.44.1 to 4.44.2 (#17) ([828adca](readmeio/syntax-highlighter@828adca)), closes [#17](readmeio/syntax-highlighter#17)
* chore(deps): bump actions/checkout from v2.3.2 to v2.3.3 (#9) ([6f1fda0](readmeio/syntax-highlighter@6f1fda0)), closes [#9](readmeio/syntax-highlighter#9)
* chore(deps): bump codemirror from 5.57.0 to 5.58.1 (#13) ([7d7c7b3](readmeio/syntax-highlighter@7d7c7b3)), closes [#13](readmeio/syntax-highlighter#13)

#### <small>9.0.1 (2020-09-16)</small>

* chore: line number dom elements updated to use span (#8) ([85aeb0e](readmeio/syntax-highlighter@85aeb0e)), closes [#8](readmeio/syntax-highlighter#8)
* fix(highlighting): fixing an issue where overlay was not applied correctly (#7) ([df2236e](readmeio/syntax-highlighter@df2236e)), closes [#7](readmeio/syntax-highlighter#7)

#### 9.0.0 (2020-09-15)

* feat: code editor export (#5) ([3c2baa0](readmeio/syntax-highlighter@3c2baa0)), closes [#5](readmeio/syntax-highlighter#5) [#6](readmeio/syntax-highlighter#6)
* docs: adding a line break in the readme ([6819099](readmeio/syntax-highlighter@6819099))

#### <small>8.0.3 (2020-09-02)</small>

* chore(deps): upgrading @readme/variable to 7.2.1 ([7ec28f3](readmeio/syntax-highlighter@7ec28f3))

#### <small>8.0.2 (2020-09-02)</small>

* docs: updating a link in the contributing docs ([c539940](readmeio/syntax-highlighter@c539940))

#### <small>8.0.1 (2020-09-01)</small>

* feat: setting up webpack (#4) ([fa651fc](readmeio/syntax-highlighter@fa651fc)), closes [#4](readmeio/syntax-highlighter#4)

#### 8.0.0 (2020-08-31)

> No breaking changes in this release, the package has just moved a new home at https://github.com/readmeio/syntax-highlighter!

* chore: moving the package code into a src/ directory ([e78554e](readmeio/syntax-highlighter@e78554e))
* chore: pulling the syntax-highlighter package over from the explorer repo ([296c7de](readmeio/syntax-highlighter@296c7de))
* chore(deps-dev): bump eslint from 7.7.0 to 7.8.0 (#2) ([9789985](readmeio/syntax-highlighter@9789985)), closes [#2](readmeio/syntax-highlighter#2)
* chore(deps-dev): bump jest from 26.1.0 to 26.4.2 (#3) ([c837dd7](readmeio/syntax-highlighter@c837dd7)), closes [#3](readmeio/syntax-highlighter#3)
* chore(deps-dev): bump prettier from 2.0.5 to 2.1.1 (#1) ([0cfbe94](readmeio/syntax-highlighter@0cfbe94)), closes [#1](readmeio/syntax-highlighter#1)
* docs: setting up changelog generation ([dcfe1db](readmeio/syntax-highlighter@dcfe1db))
* ci: adding a ci workflow for running tests ([fe631bd](readmeio/syntax-highlighter@fe631bd))
* ci: setting up dependabot ([a293e53](readmeio/syntax-highlighter@a293e53))
* test: fixing broken tests ([6257159](readmeio/syntax-highlighter@6257159))
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.

6 participants