Skip to content

Fix and Improve Ground Control - #2881

Closed
RichDom2185 wants to merge 15 commits into
masterfrom
fix-gc-table
Closed

RichDom2185 wants to merge 15 commits into
masterfrom
fix-gc-table

Conversation

@RichDom2185

Copy link
Copy Markdown
Member

Description

The mutable state should have been migrated to use useRef when the component was transitioned to a functional component.

This was a regression introduced in #2875.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

It should now work, aka fixes this bug:

telegram-cloud-photo-size-5-6059905916867819461-y

Checklist

  • I have tested this code
  • I have updated the documentation

The mutable state should have been migrated to use `useRef` when the
component was transitioned to a functional component.
@RichDom2185
RichDom2185 requested a review from jayjay19630 March 26, 2024 16:59
@RichDom2185 RichDom2185 self-assigned this Mar 26, 2024
@coveralls

coveralls commented Mar 26, 2024 •

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 9052848283

Details

  • 0 of 29 (0.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 31.581%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/pages/academy/academyRoutes.tsx 0 1 0.0%
src/pages/academy/groundControl/GroundControl.tsx 0 28 0.0%
Files with Coverage Reduction New Missed Lines %
src/pages/academy/groundControl/GroundControl.tsx 1 0.0%
Totals Coverage Status
Change from base Build 9052785407: -0.01%
Covered Lines: 4921
Relevant Lines: 14701

💛 - Coveralls

@jayjay19630

Copy link
Copy Markdown
Contributor

For some reason the table still looks the way it does. We might need to take a closer look at it
Screenshot 2024-03-27 at 1 18 59 AM

@RichDom2185

Copy link
Copy Markdown
Member Author

For some reason the table still looks the way it does. We might need to take a closer look at it Screenshot 2024-03-27 at 1 18 59 AM

Hmm, I see, alright, I'll take a closer look over the next few days. Thanks!

@RichDom2185
RichDom2185 marked this pull request as draft March 26, 2024 17:20
@RichDom2185 RichDom2185 changed the title Fix Ground Control table size Fix and Improve Ground Control Mar 28, 2024
@RichDom2185

Copy link
Copy Markdown
Member Author

For some reason the table still looks the way it does. We might need to take a closer look at it Screenshot 2024-03-27 at 1 18 59 AM

The issue is now fixed in #3001. But I'm keeping this PR open for some further improvements.

martin-henz added a commit that referenced this pull request Mar 6, 2026
The mutable gridApi variable was lost on every re-render after the
component was converted to a functional component. Migrate to useRef
to persist the reference correctly.

Also removes the Redux connect() container (GroundControlContainer)
in favour of calling useDispatch() directly in the component, consistent
with the rest of the codebase.

Revives #2881

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@martin-henz

Copy link
Copy Markdown
Member

Superseded by #3637

@martin-henz martin-henz closed this Mar 6, 2026
@RichDom2185

Copy link
Copy Markdown
Member Author

Will close if actually superseded

@RichDom2185 RichDom2185 reopened this Mar 9, 2026
@RichDom2185
RichDom2185 removed the request for review from jayjay19630 March 9, 2026 20:07
RichDom2185 added a commit that referenced this pull request May 13, 2026
…iner (#3637)

* Fix Ground Control grid API regression by migrating to useRef

The mutable gridApi variable was lost on every re-render after the
component was converted to a functional component. Migrate to useRef
to persist the reference correctly.

Also removes the Redux connect() container (GroundControlContainer)
in favour of calling useDispatch() directly in the component, consistent
with the rest of the codebase.

Revives #2881

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix infinite re-render loop in Ground Control

Two causes identified:
- onGridSizeChanged calling sizeColumnsToFit() created a feedback loop
  (resize → sizeColumnsToFit → resize → ...). Removed the handler.
- columnDefs being redefined on every render caused AG Grid to
  re-process columns each cycle. Wrapped in useMemo([dispatch]) so it
  is only computed once (dispatch from useDispatch is stable).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: format GroundControl.tsx

* Fix most errors post-merge

* Fix error post-merge

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants