Skip to content

Conversation

@alesan99
Copy link
Contributor

@alesan99 alesan99 commented Sep 8, 2025

Fixes #7322

Some types changed so some minor code changes were needed.
Upgraded @testing-library/react too because multiple tests were suddenly failing (because user events weren't being automatically wrapped in act() ).

Checklist

  • Self-review the PR after opening it to make sure the changes look good and
    self-explanatory (or properly documented)
  • Add relevant issue to release milestone
  • Add pr to documentation list
  • Add automated tests

Testing instructions

  • Go to a workbench dataset
  • Make sure no new errors occur when adding and modifying data on the spreadsheet.
  • Make sure scrolling/navigation works as expected when you have a lot of rows or columns. (Like this dataset: FishTissueData.tsv )
  • Make sure workbench performance/speed when scrolling isn't worse than in main.
  • Make sure you can successfully validate and upload a dataset.

@alesan99 alesan99 changed the title Bump dompurify and handsontable Bump dompurify, handsontable, and @testing-library/react Sep 10, 2025
@alesan99 alesan99 marked this pull request as ready for review September 10, 2025 16:36
callback: handleClickDisambiguate,
},
['separator_1' as 'undo']: '---------',
separator_1: '---------',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

['separator_1' as 'undo'] was showing me an error, so I changed it to just separator_1.
It was originally done to avoid a type error in the first place 3d3eb1d, but it seems to be fine now, I hope.

@alesan99 alesan99 requested review from a team September 10, 2025 16:41
@alesan99 alesan99 added this to the 7.12.0 milestone Sep 10, 2025
@CarolineDenis CarolineDenis removed request for a team September 10, 2025 18:43
@CarolineDenis CarolineDenis requested review from a team and acwhite211 September 10, 2025 19:03
@CarolineDenis CarolineDenis modified the milestones: 7.12.0, 7.11.2 Sep 10, 2025
Copy link
Collaborator

@bhumikaguptaa bhumikaguptaa left a comment

Choose a reason for hiding this comment

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

  • Make sure no new errors occur when adding and modifying data on the spreadsheet.
  • Make sure scrolling/navigation works as expected when you have a lot of rows or columns.
  • Make sure you can successfully validate and upload a dataset.

Looks good!

@bhumikaguptaa bhumikaguptaa requested a review from a team September 10, 2025 19:38
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Make sure no new errors occur when adding and modifying data on the spreadsheet.
  • Make sure scrolling/navigation works as expected when you have a lot of rows or columns.
  • Make sure you can successfully validate and upload a dataset.

Scrolling behavior is a bit weird and rows/columns seem to constantly change size.

Main:

09-10_14.54.mp4

Issue branch:

09-10_14.55.mp4

@github-project-automation github-project-automation bot moved this from 📋Back Log to Dev Attention Needed in General Tester Board Sep 10, 2025
@CarolineDenis CarolineDenis modified the milestones: 7.11.2, 7.12.0 Sep 11, 2025
@alesan99
Copy link
Contributor Author

alesan99 commented Sep 22, 2025

@emenslin I tweaked a couple of things to make rows load more reliably and to make columns resize correctly (Maybe better than it was before).
I'm not super proud of my solution though, it may have an impact on performance, so I'd like another @specify/ux-testing testing check just to be safe.

Dev note:
I am manually triggering a column size recalculation and table render after scrolling. Also I increased the row loading buffer, which fixes rows partially loading (For most screen sizes presumably). I feel like these might have a noticeable performance impact on lower-end systems.
Trying to downgrade to v15 still has the same issues, and v14 reintroduces vulnerabilities which we are trying to avoid in the first place by upgrading.

@alesan99 alesan99 requested review from a team and emenslin September 22, 2025 16:15
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Make sure no new errors occur when adding and modifying data on the spreadsheet.
  • Make sure scrolling/navigation works as expected when you have a lot of rows or columns. (Like this dataset: FishTissueData.tsv )
  • Make sure workbench performance/speed when scrolling isn't worse than in main.
  • Make sure you can successfully validate and upload a dataset.

Scrolling behavior is now lagging and sometimes does not render the rows.

Issue branch:

09-22_12.20.mp4

Main:

09-22_12.21.mp4

@CarolineDenis CarolineDenis modified the milestones: 7.12.0, 7.13.0 Sep 22, 2025
@alesan99
Copy link
Contributor Author

Hmm okay after some consideration it might be best to wait for handsontable to possibly address some of the rendering issues.

It looks like autoColumnSize is broken on virtualized tables, and trying to manually fix it makes it laggy so I'm not sure there's a good solution right now.

This PR can be repurposed once a new update is out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Dev Attention Needed

Development

Successfully merging this pull request may close these issues.

Bump dompurify and handsontable

5 participants