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.

Sign up for GitHub

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

Jump to bottom

Convert GridMetricCalculator, GridRange, GridUtils to TypeScript #302

Merged
merged 13 commits into from
Nov 25, 2021
Merged

Convert GridMetricCalculator, GridRange, GridUtils to TypeScript #302

merged 13 commits into from
Nov 25, 2021

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Nov 18, 2021 •
edited
Loading

@mofojed mofojed added this to the November 2021 milestone Nov 18, 2021
@mofojed mofojed self-assigned this Nov 18, 2021
spasovski
spasovski previously approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@spasovski spasovski left a comment

Choose a reason for hiding this comment

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

Looks good. Glad this is moving forward.

mattrunyon
mattrunyon requested changes Nov 18, 2021
View reviewed changes
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetrics.ts Outdated Show resolved Hide resolved
packages/grid/src/GridUtils.ts Outdated Show resolved Hide resolved
packages/grid/src/GridUtils.ts Outdated Show resolved Hide resolved
packages/grid/src/GridUtils.ts Outdated Show resolved Hide resolved
packages/grid/src/GridUtils.ts Outdated Show resolved Hide resolved
@mofojed
Clean up from review
a1c1552 
- Correct 'eg.' to 'e.g.'
- Renamed Index to VisibleIndex
- Renamed itemXs to itemCoordinates to be a little more accurate
@mofojed
More clean up from review
8d50bda 
Missed a few comments the first time
mattrunyon
mattrunyon requested changes Nov 19, 2021
View reviewed changes
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
@mofojed
WIP clean up GridMetricCalculator a bit
1bea433
@mofojed
WIP Throw if values aren't set... some values aren't being set
320f5fa
@mofojed
Clean up some calculations
c20ac26 
- Now that it throws if we're trying to retrieve a value that's not in the map, needed a couple more fixes.
- Add the floating rows/columns heights to the height/column maps right away, before we calculate all the positions
- Check if columnCount or rowCount is 0 before getting some values
@mofojed mofojed requested a review from mattrunyon November 22, 2021 19:05
mattrunyon
mattrunyon requested changes Nov 23, 2021
View reviewed changes
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Outdated Show resolved Hide resolved
packages/grid/src/GridMetricCalculator.ts Show resolved Hide resolved
@mofojed mofojed requested a review from mattrunyon November 24, 2021 16:05
@mofojed mofojed mentioned this pull request Nov 24, 2021
Closed
mattrunyon
mattrunyon requested changes Nov 24, 2021
View reviewed changes
Copy link
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Change mapGet to be called something like getOrThrow or mapGetOrThrow to make it clear to anybody in the future that it can throw

Remove default value if you want

@mofojed mofojed requested a review from mattrunyon November 24, 2021 22:43
@mofojed mofojed merged commit 3b46b7a into deephaven:main Nov 25, 2021
@mofojed mofojed deleted the 21-grid-js-to-ts branch November 25, 2021 14:38
mofojed added a commit to mofojed/web-client-ui that referenced this pull request Nov 26, 2021
@mofojed
Revert "Convert GridMetricCalculator, GridRange, GridUtils to TypeScr…
a19028b 
…ipt (deephaven#302)"

This reverts commit 3b46b7a.
mofojed added a commit that referenced this pull request Nov 26, 2021
@mofojed
Revert "Convert GridMetricCalculator, GridRange, GridUtils to TypeScr…
676fd71 
…ipt (#302)" (#313)

This reverts commit 3b46b7a.
mofojed added a commit to mofojed/web-client-ui that referenced this pull request Nov 26, 2021
@mofojed
Convert GridMetricCalculator, GridRange, GridUtils to TypeScript (dee…
939349d 
…phaven#302)

- Convert part of grid to TypeScript 
- Add some JSDocs
@mofojed mofojed mentioned this pull request Nov 26, 2021
Merged
mofojed added a commit that referenced this pull request Dec 1, 2021
@mofojed
rerevert grid cleanup (#317)
86f5e91 
- Convert GridMetricCalculator, GridRange, GridUtils to TypeScript (#302)
- Add some JSDocs
- Fix the calculation for visibleRowHeights/visibleColumnWidths. Fixes #307 and #308 
- TODO followup for more cleanup with #316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattrunyon mattrunyon mattrunyon approved these changes

@spasovski spasovski Awaiting requested review from spasovski

Assignees

@mofojed mofojed

Labels
internal web-client-ui
Projects
None yet
Milestone
November 2021
Development

Successfully merging this pull request may close these issues.
3 participants
@mofojed @spasovski @mattrunyon