More flexible "External viewer actions" #3262
Conversation
…ctions in some cases
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
🦋 Changeset detectedLatest commit: e145e93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| type Props = PropsWithChildren<{ | ||
| partialPlaygroundSettings: PartialPlaygroundSettings; | ||
| editor?: CodeEditorHandle; | ||
| externalViewerActions?: ExternalViewerActions; |
There was a problem hiding this comment.
The prop name is intentionally verbose. At first I tried to shorten it to actions or externalActions when it was obvious from context, but it made things less readable and greppable.
| }>; | ||
|
|
||
| // Configure external actions for the common case of editor actions, e.g. for the playground or `<SquiggleEditor>` component. | ||
| export function useExternalViewerActionsForEditor( |
There was a problem hiding this comment.
I want to split ViewerProvider into multiple files, but didn't want to complicate the diff for this PR.
|
This seems better in most cases, e.g. long dicts that are below the current viewport in editor will scroll into range: One awkward corner case here is that if the dict or a function is very long, and won't fit in the viewport, Codemirror will scroll to the end of it even if this means that the beginning of the selection won't be in the viewport. This is a consequence of selection "head" (cursor position) being at the end. But this is probably fine. |
| await waitFor(() => screen.getByText(/simulation/)); | ||
|
|
||
| expect(screen.getByText(/simulation/)).toHaveTextContent( | ||
| /simulation #(\d+) in (\d+)ms/ |
There was a problem hiding this comment.
minor, but I'd prefer it if /simulation and /simulation #(\d+) in (\d+)ms/ were pulled out as constants.
|
This seems very reasonable, let's go with it for now. |
We now pass
externalViewerActionstoViewerProviderinstead of an editor object, becauseeditor.scrollTodoesn't make sense in some non-playground contexts, e.g. GUCEM.To avoid boilerplate, there's a
useExternalViewerActionsForEditorhook which converts the editor handle to theexternalViewerActions.Also, the viewer behavior regarding these actions is now more flexible and has a better support for imports, thanks to the two new methods:
isFocusableandisDefaultSourceId.Example screenshot from the modified story; the error here comes from the function defined in an import, and the line in imported file is not clickable (for now), but the top-level call location is clickable.
I didn't commit this story to the repo, but I did implemented some basic tests.