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

fix: sheet getRowVisible lags after filter #3396

Merged
merged 4 commits into from
Sep 12, 2024
Merged

Conversation

lumixraku
Copy link
Contributor

@lumixraku lumixraku commented Sep 10, 2024

close #3369

Pull Request Checklist

  • Related tickets or issues have been linked in the PR description (or missing issue).
  • Naming convention is followed (do please check it especially when you created new plugins, commands and resources).
  • Unit tests have been added for the changes (if applicable).
  • Breaking changes have been documented (or no breaking changes introduced in this PR).

@lumixraku lumixraku force-pushed the fix/sheet-filter-row branch 2 times, most recently from 66fb2da to c0c2bf1 Compare September 11, 2024 03:04
Copy link

github-actions bot commented Sep 11, 2024

View Deployment

📑 Examples 📚 Storybook
🔗 Preview link 🔗 Preview link

@lumixraku lumixraku force-pushed the fix/sheet-filter-row branch 2 times, most recently from 7a9fdae to 937c8d9 Compare September 11, 2024 03:15
@lumixraku lumixraku marked this pull request as ready for review September 11, 2024 03:23
@univer-bot univer-bot bot added the qa:untested This PR is ready to be tested label Sep 11, 2024
@lumixraku lumixraku changed the title fix: sheet filter row fix: sheet getRowVisible lags after filter Sep 11, 2024
}
return !!this._filteredRowCacheMap.get(row);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key changes, add cache for isRowFiltered, intercetor slow down performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

是interceptor的自身逻辑导致的性能问题?判断是否是filter行,我看底层就是set.has这样一个逻辑,看起来这个并不是一个高耗时的逻辑。
这个cache的设计,看起来就是单纯为了避免走一次 interptor 自身的逻辑?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rollback 了

Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 131 lines in your changes missing coverage. Please review.

Project coverage is 30.45%. Comparing base (5f2e9fa) to head (531c945).
Report is 5 commits behind head on dev.

Files with missing lines Patch % Lines
...ine-render/src/components/sheets/sheet-skeleton.ts 0.00% 71 Missing ⚠️
packages/engine-render/src/viewport.ts 0.00% 19 Missing ⚠️
packages/engine-render/src/context.ts 0.00% 10 Missing ⚠️
...ne-render/src/components/sheets/extensions/font.ts 0.00% 7 Missing ⚠️
...-render/src/components/sheets/extensions/custom.ts 0.00% 4 Missing ⚠️
packages/core/src/sheets/worksheet.ts 0.00% 3 Missing ⚠️
...-render/src/components/sheets/extensions/marker.ts 0.00% 3 Missing ⚠️
...nditional-formatting/src/render/data-bar.render.ts 0.00% 3 Missing ⚠️
...s-conditional-formatting/src/render/icon.render.ts 0.00% 3 Missing ⚠️
...s/permission/extensions/range-protection.render.ts 0.00% 3 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3396      +/-   ##
==========================================
- Coverage   30.50%   30.45%   -0.06%     
==========================================
  Files        2117     2126       +9     
  Lines      110602   110795     +193     
  Branches    24153    24180      +27     
==========================================
  Hits        33742    33742              
- Misses      76860    77053     +193     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chore: revert rowcol visible back
@zhaolixin7 zhaolixin7 added the qa:verified This PR has already by verified by a QA and is considered good enough to be merge label Sep 12, 2024
@univer-bot univer-bot bot removed the qa:untested This PR is ready to be tested label Sep 12, 2024
@dream-num dream-num deleted a comment from univer-bot bot Sep 12, 2024
@lumixraku lumixraku merged commit cb2cf7e into dev Sep 12, 2024
16 checks passed
@lumixraku lumixraku deleted the fix/sheet-filter-row branch September 12, 2024 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa:verified This PR has already by verified by a QA and is considered good enough to be merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Horizontal scroll freeze/hang on filtering large data If the number of rows in the filter is low
4 participants