jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
596 stars 127 forks source link

Datagrid: Introduce AsyncCellRenderer and ImageRenderer #630

Closed martinRenou closed 9 months ago

martinRenou commented 9 months ago

This is a step towards fixing https://github.com/bloomberg/ipydatagrid/issues/253

Introducing AsyncCellRenderer

Prior to this change, the Lumino datagrid would not allow rendering cells asynchronously. This means that rendering images was not really possible (unless the cell renderer had a reference to the datagrid and requested a redraw once the asynchronous task was done).

This PR introduces the AsyncCellRenderer abstract class, which the datagrid uses as following (more or less pseudo-code):

if (renderer instanceof AsyncCellRenderer) {
  if (!renderer.isReady(config)) {
    // Paint a placeholder, waiting for all asynchronous tasks to be performed
    renderer.paintPlaceholder(gc, config);

    // Perform all asynchronous tasks (without waiting) then redraw the region of the cell
    renderer.load(config).then(() => {
      this.repaintRegion(cell.position);
    }); 
  } else {
    // The async renderer is ready, we can paint synchronously
    renderer.paint(gc, config);
  }
} else {
  // Classic synchronous painting
  renderer.paint(gc, config);
}

This allows supporting asynchronous cell rendering without slowing down the synchronous way.

Introducing the ImageRenderer

This implements the AsyncCellRenderer and allows to load images in the datagrid:

Screenshot from 2023-09-06 13-59-48

martinRenou commented 9 months ago

Ready for review!! CI failure seems unrelated, I don't have the right to restart it

martinRenou commented 9 months ago

All green! Thanks to the person who restarted it :D

martinRenou commented 9 months ago

@krassowski thanks a lot for the useful review!

I just pushed another commit:

martinRenou commented 9 months ago

Also, we now have a minimal scaffold for datagrid tests here, it would be greatly appreciated if the critical subset of the new API was unit-tested

Oops forgot to tackle this, will do

martinRenou commented 9 months ago

@krassowski I think it's good for another round of review if you have time!