Skip to content

fix: Grid rendering header incorrectly when hiding all children in a group via layout hints#1139

Merged
mattrunyon merged 3 commits intodeephaven:mainfrom
mattrunyon:layout-hint-hide-group
Mar 8, 2023
Merged

fix: Grid rendering header incorrectly when hiding all children in a group via layout hints#1139
mattrunyon merged 3 commits intodeephaven:mainfrom
mattrunyon:layout-hint-hide-group

Conversation

@mattrunyon
Copy link
Copy Markdown
Collaborator

Fixes #1097

Also updated the readme for updating e2e snapshots and removed some env variables that shouldn't be needed any more when updating/running e2e tests

@mattrunyon mattrunyon added the bug Something isn't working label Mar 7, 2023
@mattrunyon mattrunyon requested a review from mofojed March 7, 2023 21:38
@mattrunyon mattrunyon self-assigned this Mar 7, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 7, 2023

Codecov Report

Merging #1139 (a6e86e1) into main (6debb74) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1139      +/-   ##
==========================================
+ Coverage   43.40%   43.41%   +0.01%     
==========================================
  Files         435      435              
  Lines       32682    32688       +6     
  Branches     8240     8244       +4     
==========================================
+ Hits        14185    14193       +8     
+ Misses      18448    18446       -2     
  Partials       49       49              
Flag Coverage Δ
unit 43.41% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/grid/src/GridRenderer.ts 50.58% <0.00%> (ø)
packages/iris-grid/src/IrisGrid.tsx 27.00% <0.00%> (-0.02%) ⬇️
...s/dashboard-core-plugins/src/panels/ChartPanel.tsx 61.55% <0.00%> (ø)
packages/iris-grid/src/IrisGridRenderer.ts 24.61% <0.00%> (+0.24%) ⬆️
packages/iris-grid/src/IrisGridUtils.ts 54.09% <0.00%> (+0.87%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment thread playwright.config.ts
// Only start the main code-studio server right now
// To test embed-grid and embed-chart, should have an array set for `webServer` and run them all separately as there's a port check
command: 'VITE_PROXY_URL=http://localhost:10000 npm run preview:app',
command: 'npm run preview:app -- -- -- --no-open', // Passing flags through npm is fun
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆


# Now build the app. We only need the code-studio built for e2e tests.
RUN VITE_CORE_API_URL=http://host.docker.internal:10000/jsapi npm run build:app No newline at end of file
RUN npm run build:app No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I dumb? Why does this work?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we were using CRA, there was no proxy server, so if we wanted to point to a JS API not reachable at /jsapi on our preview server, we needed to build it w/ a URL instead.

Prior to #1102 we only proxied requests in dev mode for Vite. With that change, we proxy requests for the Vite preview server as well. So now we can build expecting /jsapi to resolve properly since it will go through the proxy which defaults to localhost:10000

@mattrunyon mattrunyon merged commit 2fbccc6 into deephaven:main Mar 8, 2023
@mattrunyon mattrunyon deleted the layout-hint-hide-group branch March 8, 2023 18:42
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hiding columns via layout hints in a group rendering bug

2 participants