Conversation
megoth
left a comment
megoth
left a comment
There was a problem hiding this comment.
A lot of improvements here, and I'm more positive to React with this. Lets try a bit more experimenting and see where they take us.
| import { View } from './view' | ||
| import { workaroundActError } from './actErrorWorkaround' | ||
|
|
||
| workaroundActError() |
There was a problem hiding this comment.
Very nice that we can do snapshot testing!
| }, | ||
| render: (subject) => { | ||
| const container = document.createElement('div') | ||
| ReactDOM.render(<Container store={store} subject={subject}/>, container) |
There was a problem hiding this comment.
Very simple connection between panes and React... very interesting
| import { View } from './view' | ||
| import { ContainerProps } from '../types' | ||
|
|
||
| export const Container: React.FC<ContainerProps> = (props) => { |
There was a problem hiding this comment.
Props should be typed of course, but looks like a reasonable way of passing data
There was a problem hiding this comment.
They are typed :) The generic passed to React.FC (i.e. ContainerProps) defines the types of the props. (I chose to extract them to a separate file here to encourage using the same API for all top-level containers - that would make it easier to potentially extract panes into separate apps, for example.)
| }) | ||
| } | ||
|
|
||
| if (phase === 'saving') { |
There was a problem hiding this comment.
I like this approach better than the conditionals in my experiment - I think this is easier to read.
|
This relates to #72 |
|
Should keep this open until we've merged it into master. |
|
@Vinnl I was going to work on a markdown pane and just found this! Any reason we should not merge it? |
|
@angelo-v IIRC this was to prototype the use of React in solid-panes, which is quite a dependency. And the other reason would be the large number of conflicts :P |
|
We included react already. My activtiystreams-pane is also react-based. So this should not be an issue. |
|
In that case I see no reason not to merge it :) |
|
Ok, I will give it a try |
|
I moved the code to a dedicated repo https://github.com/solid/markdown-pane |
3 participants
This is strongly based on #116. I created a Markdown pane with practically the same functionality and a similar architecture as in that PR, but this time using React. This primarily demonstrates that it is possible to use React in a single pane, without having to share a data model with the rest - it simply attaches to a DOM node, which is then provided to the OutlineManager.
The primary change is that there is no controller here. That said, there still is a "Container" component, which glues the data fetching to the view (ie hard to test, but logic that's not too complicated). That means that the view logic is relatively easy to test - an example is included.
I'll look at porting the TrustedApplications Pane next to get a feel for how this would work for a more involved Pane, but thought I'd submit this to give an initial view of what this might look like.
Fixes #72.