Workspace/WorkspaceView API changes #1
Conversation
|
Could |
|
The text is aligned right now, would using markdown gives us more than that? |
|
It would render in prose diff |
| ::getActivePane() 85 51 2 | ||
| ::eachEditor(callback) 55 38 4 | ||
| ::registerOpener(opener) 41 40 10 | ||
| ::prependToBottom(element) 34 31 3 |
There was a problem hiding this comment.
For the prependTo... and appendTo... methods we need to decide what type of object we want to pass in.
There was a problem hiding this comment.
I think we should refer to these objects as "panels". The contract for attaching them should be the same as for pane items, and I think we should support the following options:
- A React component.
- A plain JS object with a
::getDOMNode()method and some lifecycle hooks that we will call. That allows people to write adaptors to whatever framework they want.
Less recommended path for flexibility backward compatibility:
- A SpacePen view / jQuery wrapper
- A raw DOM element.
|
I'll make a branch that uses markdown to see if the the diffs look better using prose diff. |
|
We can render it in markdown and view the diffs like this https://github.com/atom/api/compare/cj-markdown-diff...cj-markdown-diff-future?name=cj-markdown-diff-future&short_path=00e71df#diff-00e71df22262087fd8ad820708997657. I don't think it is any more readable, but if other people do I'll switch to it. |
|
I was thinking a table would be nicer than a list, since there are columns |
|
I think the txt format that's currently there is fine. There are already columns. |
| ::focusPaneOnRight() 0 0 1 | ||
| ::prependToLeft(element) 0 0 0 | ||
| ::prependToRight(element) 0 0 0 | ||
| ::focusPaneAbove() 0 0 1 |
There was a problem hiding this comment.
We should be consistent about focus and activate and rename the focusPane... or activate..Pane methods.
There was a problem hiding this comment.
If we go back to only exposing the view, focus could be a more recognizable concept. But the "active" pane is definitely different from the focused pane in that it's sticky. We record the most recently focused pane as active.
There was a problem hiding this comment.
I could also get behind removing the focus and activate pane methods.
There was a problem hiding this comment.
Should they be methods on Pane probably so they're relative to some reference point? Or maybe just providing a standard way of getting the pane above/below/to-left/to-right and then you can do whatever you want with it.
|
@nathansobo were there other changes you wanted to make to Workspace/WorkspaceView? |
| @@ -535,50 +535,30 @@ ThemeManager called packages a | |||
| ::getUserStylesheetPath() 0 0 0 | |||
| ::requireStylesheet(stylesheetPath, type, htmlElement) 0 0 0 | |||
|
|
|||
| WorkspaceView called packages atom packages | |||
| ::getActiveView() 96 61 8 | |||
| ::eachEditorView(callback) 89 79 10 | |||
There was a problem hiding this comment.
After unifying view and model, it would be great to just support onEachEditor.
| ::unregisterOpener(opener) 3 3 1 | ||
| ::saveActivePaneItem() 2 2 0 | ||
| ::destroyActivePaneItem() 1 1 0 | ||
| ::destroyActivePane() 1 1 0 |
There was a problem hiding this comment.
I replaced this with Pane::destroy
There was a problem hiding this comment.
Agree with this one. These can just be methods on the actual pane / item.
|
I'd still like to work out how we query / subscribe to different kinds of "pane items". I'd also like to float the idea of renaming "pane items" to "editors" and referring to editors for text as "text editors". It's way less awkward terminology than "pane item" and a lot more specific than "view". The fact that some editors are read-only seems like an acceptable academic detail to me. I think the fact that avocode is already building an image editing system on top of this framework bolsters the idea that we shouldn't overly specialize on terminology that assumes text editing out of the gate. |
activatePreviousPane is used by 3 packages, but it can be easily fixed: panes = atom.workspace.getPanes() activePane = atom.workspace.getActivePane() previousIndex = Math.max(0, panes.indexOf(activePane) - 1) panes[previousIndex].activate()
probablycorey
changed the title
This is a potential API for atom.workspace. The current methods for Workspace and WorkspaceView are combined to create a single more approachable API. Underused API methods are also removed or updated.