CellEditors now render in front of the DataGrid

[kgoo124]

Hi, my name is Kalen, and I am one of the Jupyter Cal Poly interns. Our team has been using the TextCellEditor class in attempts to create an editable DataGrid for the extension my team is working on, Tabular Data Editor. However, the cell editor renders behind the DataGrid, and I think this is because the updatePosition method in CellEditor uses the styles top and left, but the position isn't set. In the Tabular Data Editor, I created a class called CSVTextCellEditor with this as the only fix and the cell editor now renders in front of the DataGrid.

You can see the changes I made to the Tabular Data Editor extension here.

I am proposing that the updatePosition method in the CellEditor class include this fix so that all cell editors can render in front of the DataGrid immediately. Our team will be using the other cell editors in the near future and it would be redundant to extend each of these cell editors for this small fix. I would like to get some feedback and hear any other ideas.

Fixes #86

[kgoo124]

Just realized that the changes submitted modified a lot of the formatting and styling. Does lumino have a precommit hook for styling like JupyterLab? Does lumino also follow prettier styling as that is what I ended up using?

[afshin]

Hi @kgoo124! Thanks for the pull request.

1. It seems like your IDE is applying a lot of linting changes to this file. Could you please turn that off so that it's easier to see the changes you are proposing. I think the lint rules you're using are different from the ones the code was originally formatted with.
2. Can the change you're making be implemented as a CSS rule instead?

[kgoo124]

Hi @afshin, would you be able to elaborate a little more on the CSS rule part. I am also currently trying to get rid of the linting changes.

[afshin]

I was going to propose adding:

.lm-DataGrid-cellEditorOccluder {
  position: absolute;
}

But I just realized we don't actually have a .css file in this package, so I take it back 😅

[kgoo124]

Could I also propose adding a precommit hook for styling similar to JupyterLab, so that these styling issues don't become a problem for future changes?

[afshin]

It would be really nice to have linting be automatic. I think the reason we haven't done it is because it's hard to configure a linter to respect and enforce the conventions already in this codebase.

[kgoo124]

Definitely, let me know if there's anything else that needs to be done for this PR. Thanks for the quick reply!

[afshin]

This looks correct to me.

cc: @mbektasbbg (just in case there is something being overlooked)

[mbektas]

@afshin @kgoo124 both of these elements already have CSS classes assigned to them with position set to absolute (https://github.com/jupyterlab/lumino/blob/master/packages/default-theme/style/datagrid.css#L37 and https://github.com/jupyterlab/lumino/blob/master/packages/default-theme/style/datagrid.css#L43). @kgoo124 did you import the datagrid.css into your custom editor?

This looks correct to me.

cc: @mbektasbbg (just in case there is something being overlooked)

[blink1073]

Thanks all, merging.

[blink1073]

Published @lumino/datagrid@0.10.0

[afshin]

@mbektasbbg Is that CSS critical to the operation of the data grid? Should a subset of the datagrid default CSS be in the datagrid package instead of default-theme? Perhaps this PR's approach isn't the best outcome.

[mbektas]

Yes @afshin there is some css in default-theme that is critical for the operation of datagrid. As you suggested we should bring those into datagrid package. I will try to spare some time to do that. This PR fixes only one of the issues when user doesn't import CSS from default-theme.

[kgoo124]

@mbektasbbg, @afshin, @blink1073 that would be really helpful! Also, I think we could add some documentation specifying the default-theme css as it wasn't clear that it had to be imported. Please let me know what I can do to help as the result of these changes will be used in the Tabular Data Editor extension that my team is working on. Thanks so much for the quick responses and helpful feedback!