Add source editor component #4419

edmundoa · 2017-12-13T16:41:15Z

This PR moves the source code editor we used in the pipelines to the web interface, so other plugins can reuse it if they need it.

I manage to make a few improvements in there, hopefully they will improve the experience of using it:

Updated to latest version of ReactAce
Fixed issue with undo command after load: Graylog2 pipeline rules: Undo without undo history erases rule graylog-plugin-pipeline-processor#224. I'm not fully satisfied with the fix, so I'll try to do a better fix and get it upstream
Allow editor to be user-resizable, fixing Rules Editor: "Rule source" too small and can't be resized graylog-plugin-pipeline-processor#229
Add read-only flag, so we can also pre-visualize code with the component
Add toolbar to make more visibles some actions that can be done in the editor. For now only copying, pasting*, undo, and redo are available.

Bonus: Add dark theme support!

*Dispatching a pasting event from JS doesn't really work for privacy reasons, so the user must trigger an action in the OS. The button therefore only shows a small tooltip indicating it can be done in the Edit menu.

edmundoa · 2017-12-13T16:42:50Z

PS: For now the dark theme is only supported when specified via props. I guess we can do something better with that once we do some other improvements 🙂

kroepke

Apart from the questions and minor glitch in the docs, me gusta!

kroepke · 2017-12-19T13:42:20Z

graylog2-web-interface/src/components/common/SourceCodeEditor.jsx

+     * `[{ row: 0, column: 2, type: 'error', text: 'Some error.'}]`
+     * The type value must be one of `error`, `warning`, or `info`.
+     */
+    annotations: PropTypes.array,


Is it worth using a PropTypes.shape here or do we not bother because this typically comes from the backend anyway?

I'm most of the time against those shape objects, as you need to define the whole schema of an object (or an array of objects in this case) in a way that is not very readable and it's easy to make you waste more time removing warnings than helping catching errors. Additionally, in this case the shape is highly dependant on ace, and they don't even have good documentation for it. For instance, the column property didn't seem to have any effect when I tested it, as all annotations appear in the gutter on the right of the editor. I also had to go to the source code to see what types were supported.
In my opinion, having an example and the vales the type property accepts is good enough.

Works for me :)

kroepke · 2017-12-19T13:45:09Z

graylog2-web-interface/src/components/common/SourceCodeEditor.jsx

+                       editorProps={{ $blockScrolling: 'Infinity' }}
+                       focus={this.props.focus}
+                       fontSize={this.props.fontSize}
+                       mode="text"


Do we want to extract this into an optional prop, too?
Might come in handy if we ever use this to display/edit JSON or other modes.

The main problem is that each mode requires an import from brace. I couldn't find a way of importing all the modes brace provides, and I'm unsure if it's even safe to do so. I can play a bit more with it and see how it goes. Otherwise I would vote for extending the component when we need it.

Yeah makes sense, let's not overcomplicated it for now.

kroepke · 2017-12-19T13:47:13Z

graylog2-web-interface/src/components/common/SourceCodeEditor.md

+                  value={code} />
+```
+
+Non-resizable editor without toolbar and with custom height and width:


States that the editor isn't resizable, when in fact it is.

That was me changing props and texts last minute 😅 Will fix it.

edmundoa · 2017-12-20T15:14:37Z

@kroepke addressed your comments and other things I saw:

Now we support JSON, Lua, Markdown, and YAML languages with the mode prop
Improved the logic to disabled copy and paste buttons
Avoided resizing editor and text selection when they don't make sense
Added a note regarding uncontrolled usages of the component, also updated examples to always keep the state themselves. React-Ace is supposed to keep the text value if you don't update it by hand, but that seems to fail in some situations

edmundoa · 2018-01-02T11:10:01Z

I rebased this against master and also removed the hack to reset the undo history. The issue was fixed upstream.

edmundoa · 2018-01-02T11:19:11Z

The failing test is unrelated to the changes, but to the new year 🎉 I'll submit another PR fixing it.

This is unfortunately not possible when setting `width` to `Infinity`, but at least can be used when it has other values.

Reset undo history when the editor is fully loaded to avoid that users clear textare performing an undo command. The problem behind this is that the editor is created empty and ReactAce does a `setValue` to set the given value for the editor. This is registered as a change by ace, so when pressing undo in an apparently not-changed input, the input is reset. Fixes Graylog2/graylog-plugin-pipeline-processor#224

Copying uses our ClipboardButton component that works without external dependencies. Pasting is currently not possible without the user triggering a system event for it, so we just show a small tooltip.

Hovering over the buttons should display a tooltip with the action they perform.

We only add support for languages that make more sense to write from the web interface.

- Do not resize editor when is not resizable - Do not get text selection when copy button cannot be used

I also noticed an error when using the component in an uncontrolled way, so I updated the examples and added a note on that regard.

This was fixed upstream in securingsincity/react-ace#345

bernd · 2018-01-10T22:18:49Z

graylog2-web-interface/src/components/common/SourceCodeEditor.jsx

+                                 onSuccess={this.focusEditor}
+                                 text={this.state.selectedText}
+                                 buttonTitle="Copy (Ctrl+C / &#8984;C)"
+                                 disabled={this.isCopyDisabled} />


This passes the isCopyDisabled function instead of executing it.

Good finding! This got broken in one of the changes I did.

bernd · 2018-01-10T22:19:08Z

graylog2-web-interface/src/components/common/SourceCodeEditor.jsx

+                                 buttonTitle="Copy (Ctrl+C / &#8984;C)"
+                                 disabled={this.isCopyDisabled} />
+                <OverlayTrigger placement="top" trigger="click" overlay={overlay} rootClose>
+                  <Button bsStyle="link" bsSize="sm" title="Paste (Ctrl+V / &#8984;V)" disabled={this.isPasteDisabled}>


This passes the isPasteDisabled function instead of executing it.

No changes needed

edmundoa added the ready-for-review label Dec 13, 2017

edmundoa added this to the 3.0.0 milestone Dec 13, 2017

ghost assigned edmundoa Dec 13, 2017

edmundoa mentioned this pull request Dec 13, 2017

Source code editor Graylog2/graylog-plugin-pipeline-processor#234

Merged

edmundoa removed their assignment Dec 13, 2017

ghost assigned edmundoa Dec 13, 2017

edmundoa removed their assignment Dec 13, 2017

kroepke self-requested a review December 19, 2017 07:56

kroepke previously requested changes Dec 19, 2017

View reviewed changes

ghost assigned edmundoa Dec 20, 2017

edmundoa force-pushed the add-source-editor branch from 1bf7adf to 66d8c0e Compare January 2, 2018 10:53

edmundoa removed their assignment Jan 2, 2018

Edmundo Alvarez added 14 commits January 2, 2018 13:10

Add react-ace dependency

eea3972

Add first version of SourceCodeEditor

4e61dab

Add read-only flag to SourceCodeEditor

ee2005f

Make SourceCodeEditor resizable

e044cb4

Do not require onChange prop

7b8d069

Allow to resize editor in all axes

42b0151

This is unfortunately not possible when setting `width` to `Infinity`, but at least can be used when it has other values.

Add property to disable ClipboardButton

b62bcaa

Add toolbar with undo/redo controls

0db23c5

Enable/disable undo and redo buttons

540b2fb

Add copy/paste buttons

93864e3

Copying uses our ClipboardButton component that works without external dependencies. Pasting is currently not possible without the user triggering a system event for it, so we just show a small tooltip.

Add support for annotations

ea97c69

Add button titles to toolbar actions

c8c54ee

Hovering over the buttons should display a tooltip with the action they perform.

Adapt copy message to macOS too

abf3a66

Edmundo Alvarez added 8 commits January 2, 2018 13:10

Improve message when clicking on paste button

f591eaa

Make source editor non-resizable

8ff42eb

Add support for multiple modes

21e74e4

We only add support for languages that make more sense to write from the web interface.

Disable copy and paste buttons for read-only editors

b50090f

Avoid some computations when disabled

cf8081b

- Do not resize editor when is not resizable - Do not get text selection when copy button cannot be used

Update examples with latest changes

df5f171

I also noticed an error when using the component in an uncontrolled way, so I updated the examples and added a note on that regard.

Upgrade react-ace to 5.9.0

33b80fa

Remove hack to reset undo history

367638d

This was fixed upstream in securingsincity/react-ace#345

edmundoa force-pushed the add-source-editor branch from 0072e38 to 367638d Compare January 2, 2018 12:11

ghost assigned edmundoa Jan 2, 2018

edmundoa removed their assignment Jan 2, 2018

bernd self-assigned this Jan 5, 2018

bernd requested changes Jan 10, 2018

View reviewed changes

Fix copy and paste disabled state

67f7b57

ghost assigned edmundoa Jan 11, 2018

bernd approved these changes Jan 11, 2018

View reviewed changes

bernd merged commit 9931802 into master Jan 11, 2018

ghost removed the ready-for-review label Jan 11, 2018

bernd deleted the add-source-editor branch January 11, 2018 10:27

bernd mentioned this pull request Jan 30, 2018

Rules Editor: "Rule source" too small and can't be resized Graylog2/graylog-plugin-pipeline-processor#229

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add source editor component #4419

Add source editor component #4419

edmundoa commented Dec 13, 2017

edmundoa commented Dec 13, 2017

kroepke left a comment

kroepke Dec 19, 2017

edmundoa Dec 19, 2017

kroepke Dec 19, 2017

kroepke Dec 19, 2017

edmundoa Dec 19, 2017

kroepke Dec 19, 2017

kroepke Dec 19, 2017

edmundoa Dec 19, 2017

edmundoa commented Dec 20, 2017

edmundoa commented Jan 2, 2018

edmundoa commented Jan 2, 2018

bernd Jan 10, 2018

edmundoa Jan 11, 2018

bernd Jan 10, 2018

Add source editor component #4419

Add source editor component #4419

Conversation

edmundoa commented Dec 13, 2017

edmundoa commented Dec 13, 2017

kroepke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmundoa commented Dec 20, 2017

edmundoa commented Jan 2, 2018

edmundoa commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment