feat: Use useWidget hook to load widgets
#502
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Simplifies a lot of the logic in WidgetHandler - Don't need to pass through the `fetch` function anymore - Haven't tested from Enterprise yet
mattrunyon
requested changes
May 31, 2024
|
e2e tests won't work until I've pushed the web-client-ui changes and deephaven-core has been updated. |
mofojed
added a commit
to deephaven/web-client-ui
that referenced
this pull request
Jun 4, 2024
- Hook for loading a widget that makes it easy to use - `ObjectManager` context allows for implementation to handle loading the object - On Core side, we have a very simple `ObjectManager`, as we just have one connection - On Enterprise side, the `ObjectManager` provided to the context manager will need to handle fetching from queries, and will be able to handle more scenarios (such as when a query is restarting) - Use with deephaven/deephaven-plugins#502
mofojed
added a commit
to mofojed/web-client-ui
that referenced
this pull request
Jun 4, 2024
- Hook for loading a widget that makes it easy to use - `ObjectManager` context allows for implementation to handle loading the object - On Core side, we have a very simple `ObjectManager`, as we just have one connection - On Enterprise side, the `ObjectManager` provided to the context manager will need to handle fetching from queries, and will be able to handle more scenarios (such as when a query is restarting) - Use with deephaven/deephaven-plugins#502
mofojed
added a commit
to deephaven/web-client-ui
that referenced
this pull request
Jun 4, 2024
- Hook for loading a widget that makes it easy to use - `ObjectManager` context allows for implementation to handle loading the object - On Core side, we have a very simple `ObjectManager`, as we just have one connection - On Enterprise side, the `ObjectManager` provided to the context manager will need to handle fetching from queries, and will be able to handle more scenarios (such as when a query is restarting) - Use with deephaven/deephaven-plugins#502
mattrunyon
requested changes
Jun 10, 2024
mattrunyon
requested changes
Jun 13, 2024
- Doesn't show the full message until you click the Info button - Reload button only visible if added as an action
- Conform WidgetError to Error type
- Also fix up error type to pass back a `name` like other errors in JS
|
Needs some max-height or shrinking on the error message. If panels are relatively small the disconnected overlay breaks out of the panel (see bottom right panels in this screenshot) Also, there is some inconsistency around some of the styling it seems between the panels and tables. The text stands out more on a table than the other panels. This might just be the styling for iris-grid though and not something we care about right now
|
|
Yea for the other panels that'll need to be taken care of when we move them to use |
- Get scrolling if it is too big for the component
mattrunyon
approved these changes
Jun 14, 2024
mofojed
added a commit
to mofojed/deephaven-plugins
that referenced
this pull request
Jun 14, 2024
- Needs deephaven/web-client-ui#2030 - Uses the `useWidget` hook defined in deephaven/web-client-ui#2030 to load widgets - Allows for the errors/fetch implementation to be defined at the top level - Needed for DH-16737 - Tested in Core with a bunch of widgets, works as expected - Unit tests added
mofojed
added a commit
that referenced
this pull request
Jun 14, 2024
- Needs deephaven/web-client-ui#2030 - Uses the `useWidget` hook defined in deephaven/web-client-ui#2030 to load widgets - Allows for the errors/fetch implementation to be defined at the top level - Needed for DH-16737 - Tested in Core with a bunch of widgets, works as expected - Unit tests added
bmingles
added a commit
that referenced
this pull request
Jun 17, 2024
Rollup and esbuild optional dependencies were removed from the package-lock by this PR: #502 Adding them back to top level `optionalDependencies` to ensure they are available For reference to get this list I: 1. used `vscode` file history to find the package-lock.json diff that removed the dependencies. 2. copy / pasted the removed blocks of `esbuild` and `rollup` dependencies into a new `.json` file 3. Used `jq` to flatten the names + versions `cat myfile.json | jq 'with_entries(.value = .value.version)'` 4. Tweak / copy / pasted into optionalDependencies resolves #560
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
useWidgethook web-client-ui#2030useWidgethook defined in feat: DH-16737 Add ObjectManager,useWidgethook web-client-ui#2030 to load widgets