Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: DH-17454: Fix Spectrum component theme in modal dialogs #2196

Closed
wants to merge 30 commits into from

Conversation

vbabich
Copy link
Collaborator

@vbabich vbabich commented Aug 20, 2024

Cherry-pick #2173 and #2169 from v0.88

mofojed and others added 30 commits July 11, 2024 21:16
- Advanced Settings section in the Settings menu, with an option to
disable WebGL
- Read from the server config for default values
  - web.webgl: default setting for WebGL being enabled or not
  - web.webgl.editable: disallow the user from enabling WebGL
- Contextual Help item to add more info
- Tested by adding the following to the deephaven.prop config:
```
web.webgl=false
web.webgl.editable=false
client.configuration.list=java.version,deephaven.version,barrage.version,http.session.durationMs,file.separator,web.storage.layout.directory,web.storage.notebook.directory,web.webgl,web.webgl.editable
```
---------

Co-authored-by: Don <dsmmcken@gmail.com>
- Broke out explicit `ItemConfig` sub types and converted `ItemConfig`
type into a union them all
- Updated `createContentItem` to be more explicit in its mapping of
config types to returned content items
- Updated references to `ItemConfigType` with `ItemConfig`
- Updated `Component`, `RowOrColumn`, and `Stack` classes to allow null
`parent`

resolves #2130
This is part of the work needed for #885 that I felt would be better as
a separate PR.

Snapshot changes are going to be slightly shifted sort icon and vertical
center + slight left shift on tree table arrows.
- Add XComponent (eXtendable Component) framework to allow components to
be wrapped/replaced
  - Similar to Swizzling in Docusaurus
- Useful for components that we need to replace at the Enterprise level
(e.g. `WidgetPanelTooltip` needs to display the query name in
Enterprise)
  - Added to the StyleGuide
- Pass a `VariableDescriptor` to `WidgetPanel` and `WidgetPanelTooltip`
- Allows for more information to be included, e.g. a
`QueryVariableDescriptor` which extends `VariableDescriptor`
- Refactor the Core plugins to be consistent in how they handle the
`PanelEvent.OPEN` event
- Will allow Enterprise to use these plugins straight up and remove
their duplicated panel code
- Add functions for `PanelEvent.OPEN`
- Instead of using `PanelEvent.OPEN` directly and not getting any type
safety, add functions for emitting, listening, and a hook to enforce
type safety
Resolves #2087 

**Steps Moving Forward:**
- Clarify whether we want to be able to **handle edits that are not in
the current viewport** -- with my understanding, this will mean we will
have to load in more grid data, which does not seem overly valuable (in
terms of the efficiency tradeoff) since we are dealing with an
improbable edge case where the user begins to edit in the viewport,
continues editing outside of the viewport where the cell originally was,
and then wants to commit those changes outside as well

** A better solution for the edge case above IMO is to **create a new
enhancement ticket** where we attempt to have the editing cell be
floating/sticky in the grid viewport, similar to how it is done on
Google Sheets **

**_Ticket Created:_**
#2153

<img width="276" alt="Screenshot 2024-07-16 at 10 37 02 AM"
src="https://github.com/user-attachments/assets/a5dd6e6e-5f91-4625-b225-ddfc46d5ea89">
Resolves #2079 ( Resolves #2066 , Resolves #2103 , Resolves #2104 ,
Resolves #2105 , Resolves #2106 , Resolves #2107 , Resolves #2108 ,
Resolves #2109 , Resolves #2049 ), Resolves #2120, Resolves #1904

**Changes Implemented:**
- Replaced `keysTable` with `baseTable` 
- Made small UI changes to differentiate between Partition Table and
Partition-aware source table
- Allow double clicking to set partition
- Add context menu action to set partition

**Sample Visuals:**
_Default Partition Table View_
<img width="798" alt="Screenshot 2024-06-26 at 2 57 09 PM"
src="https://github.com/deephaven/web-client-ui/assets/69530774/19059a9f-7e69-470a-818e-c1d6d9ce8118">
_Default Partition-aware source table view:_
<img width="789" alt="Screenshot 2024-06-26 at 2 56 59 PM"
src="https://github.com/deephaven/web-client-ui/assets/69530774/af891a26-3395-4842-963f-c8b2f1c9186d">
Closes #2093 (For now, will reopen ticket after merge for better
solution to handle deprecated `panel` prop)
- We creating a new session and getting the session details, even though
that was unnecessary for fetching the object itself since we already
have a connection
- Just pass `isConsoleAvailable: false` here, since we will never have
the console available in the embedded widget scenario
- Should speed up the embed-widget fetch quite a bit
… code (#2157)

Enabled `@typescript-eslint/return-await` eslint rule and fixed
offending code.

fixes #2154

BREAKING CHANGE: Fix any try / catch blocks that return non-awaited
Promises
Fixes #885 

This adds a user setting for grid density in the theme section which
applies to all grids without an explicit density prop. A separate dh.ui
PR will allow setting the density per table and is not influenced by the
global density setting.
- If it's not available, just fallback to the getKeyTable API (which we
were using before)
- Tested against deephaven-core 0.35.1
```
from deephaven import new_table

x = new_table({
    "Foo": [0, 1, 2, 0, 1, 2],
    "Bar": [4, 5, 6, 7, 8, 9]}
).partition_by(["Foo"])
```
Wrap the `Modal` component attached to `document.body` in
`SpectrumThemeProvider`.

Had to add `zIndex: 0` and `position: relative` to the
`SpectrumThemeProvider` to fix the issue with `DatePicker` popovers
nested in the `Modal` rendering behind it.

![Screenshot 2024-07-23 at 4 35
35 PM](https://github.com/user-attachments/assets/1a860697-f4dd-4565-bf0e-6269f24a8723)
Needed for deephaven/deephaven-plugins#657
because when using ChartPanel `containerRef` is undefined on first
render due to the loading logic. This means we need to allow a ref
callback so we can use a callback to set state in the plugin and add
event listeners to the container when it actually exists
I ran `npm i` to sync package-lock with latest package.json. Fixed a
mismatched version number for `@internationalized/date`

fallout from #2170
This is only an issue if we directly set a public member. In the current
proxy model, we only set via `setField` type functions, so things seem
to be fine. Inside of class definitions the proxy is skipped (since the
proxy is just returned from the ctor).

This change makes setting a member defined in the proxy class work even
if you try to do it on the proxy object.

```ts
class ProxyModel {
  myField = 4
}

test = new ProxyModel();
test.myField; // 4
test.model.myField; // undefined

test.myField = 5;
test.myField; // 4 - WRONG
test.model.myField; // 5
```
- Split ConsoleMenu up so objects is it's own component
ConsoleObjectsMenu
- Convert to functional component
- Display all widgets in the menu
- Add a flag to hide the menu, which we set for DHC, but will not be set
in DHE so current behaviour is not broken there
- Fixes #1884

---------

Co-authored-by: Matthew Runyon <matthewrunyon@deephaven.io>
Errors thrown during a `requestAnimationFrame` are not caught because
they're thrown from a separate stack than the component. This means any
render method that throws after the first grid render won't actually be
caught and the grid will potentially draw incorrectly.
@vbabich vbabich closed this Aug 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants