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

feat: Adjustable grid density and icon size #2331

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jan 6, 2025

This is part of the work needed for deephaven#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.
Fixes deephaven#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.
@mofojed mofojed requested a review from mattrunyon January 6, 2025 19:53
@mofojed mofojed self-assigned this Jan 6, 2025
…n#2187)

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
```
@mofojed mofojed merged commit d8d04fc into deephaven:release/v0.85 Jan 6, 2025
11 checks passed
@mofojed mofojed deleted the v0.85-grid-density branch January 6, 2025 21:07
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 2025
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.

2 participants