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

Multi-cell selection of hundreds of cells takes ages to show up #6568

Closed
oleq opened this issue Apr 7, 2020 · 5 comments
Closed

Multi-cell selection of hundreds of cells takes ages to show up #6568

oleq opened this issue Apr 7, 2020 · 5 comments
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:table resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:discussion status:stale type:performance This issue reports a performance issue or a possible performance improvement.

Comments

@oleq
Copy link
Member

oleq commented Apr 7, 2020

📝 Provide a description of the improvement

  1. Go to https://en.wikipedia.org/wiki/List_of_countries_and_dependencies_by_population.
  2. Copy and paste the big countries table (243 rows in total).
  3. Select some cell at the beginning of the table.
  4. Scroll to the end (or near the end).
  5. SHIFT+Click to create a huge selection across ~200 rows.

Expected: That's a lot of cells, I get it but the acceptable delay is 1s tops.

Actual: Around ~3s on a i7 MacBook. That's a lot.

Funny fact: De-selecting (e.g. clicking in a cell to create a collapsed selection) is lightning fast. So it's not about rendering but about calculating the selection and it should be simpler to improve.

It looks like the selection.setRanges() is the performance hog

image

image

because we pass billions of ranges there. Maybe instead of

<row>[<cell />][<cell />][<cell />][<cell />][<cell />]</row>

we could at least

<row>[<cell /><cell /><cell /><cell /><cell />]</row>

to limit the number of ranges passed to the engine?

@jodator @Reinmar


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Reinmar
Copy link
Member

Reinmar commented Apr 21, 2020

The outline is a mess:

There's definitely something wrong on convertSelection() (the middle part):

But then we have rendering which isn't better because it causes hundreds of reflows:

@Reinmar
Copy link
Member

Reinmar commented Apr 21, 2020

I'd say that it nicely shows at least 3 or 4 issues and I don't expect one quick fix here. For instance, selecting multiple cells in one range like @oleq proposed will not fix the rendering which is per-cell.

@Reinmar
Copy link
Member

Reinmar commented Apr 21, 2020

As for the rendering part – my guess is that it may be a regression caused by the table selection overlay introduced recently.

@Reinmar Reinmar added this to the iteration 32 milestone Apr 21, 2020
@Reinmar Reinmar added the domain:ui/ux This issue reports a problem related to UI or UX. label Apr 21, 2020
@Reinmar Reinmar self-assigned this May 6, 2020
@Reinmar Reinmar modified the milestones: iteration 32, next, iteration 33 May 25, 2020
@Reinmar Reinmar modified the milestones: iteration 33, next Jun 18, 2020
@jodator jodator added squad:dx squad:core Issue to be handled by the Core team. labels Jul 28, 2020
@Reinmar Reinmar added squad:core Issue to be handled by the Core team. and removed squad:core Issue to be handled by the Core team. labels Oct 26, 2020
@Reinmar Reinmar removed their assignment Oct 26, 2020
@pomek pomek modified the milestones: next, nice-to-have Nov 10, 2020
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 12, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:table resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:discussion status:stale type:performance This issue reports a performance issue or a possible performance improvement.
Projects
None yet
Development

No branches or pull requests

5 participants