Missing styles in default theme

[sopsick]

The Lumino code appears to be switching the class naming conventions to lm-name_ rather than the Phosphor p-_name_ style. The @lumino/default-theme repo has not been updated with the new names. Specifically I noticed that the datagrid.css file is missing the styles that allow the cellEditors to work properly.

[jasongrout]

Thanks! Do you want to put in a PR fixing this?

[afshin]

It looks like these issues came in with cell editing changes. One thing that also needs to be addressed is the .invalid class, which should be .lm-mod-invalid I imagine.

[jasongrout]

CC also @mbektasbbg

[jasongrout]

One thing that also needs to be addressed is the .invalid class, which should be .lm-mod-invalid I imagine.

+1. As a general rule, all css classes should be namespaced, which in css means prefixed :(.

[sopsick]

Thanks! Do you want to put in a PR fixing this?

Looking at the git repo everything looks fine. It's the @lumino/default-theme in npm that has the problem. I have no idea how to update that. I think you just need to publish a new version.

[afshin]

Wait, is this actually a problem at all?

There was never a cell editor in the Phosphor data grid. So there should be no backward incompatibility with the new classes that have been created. (The .invalid class name still needs to be lm-mod-invalid, though.)

[afshin]

I don't think this is an actual backward-incompatible issue, because no consumer of @phosphor/datagrid ever had these new lm-* classes. If I am misunderstanding, please let me know. Otherwise, I propose closing this issue.

[jasongrout]

Looking at the git repo everything looks fine. It's the @lumino/default-theme in npm that has the problem. I have no idea how to update that. I think you just need to publish a new version.

According to this, it sounds like everything actually is fine in master...

except for:

(The .invalid class name still needs to be lm-mod-invalid, though.)

[sopsick]

Yes everything is fine in master but people using the Lumino packages from npm will have some issues.

[afshin]

This is the datagrid.css file I see in the NPM package for @lumino/default-theme version 0.2.4. It looks correct to me. It is identical to master.

/*-----------------------------------------------------------------------------
| Copyright (c) 2014-2018, PhosphorJS Contributors
|
| Distributed under the terms of the BSD 3-Clause License.
|
| The full license is in the file LICENSE, distributed with this software.
|----------------------------------------------------------------------------*/


/* <DEPRECATED> */ .p-DataGrid, /* </DEPRECATED> */
.lm-DataGrid {
  min-width: 64px;
  min-height: 64px;
  border: 1px solid #A0A0A0;
}


/* <DEPRECATED> */ .p-DataGrid-scrollCorner, /* </DEPRECATED> */
.lm-DataGrid-scrollCorner {
  background-color: #F0F0F0;
}


/* <DEPRECATED> */ .p-DataGrid-scrollCorner::after, /* </DEPRECATED> */
.lm-DataGrid-scrollCorner::after {
  content: '';
  position: absolute;
  top: 0;
  left: 0;
  width: 1px;
  height: 1px;
  background-color: #A0A0A0;
}

.lm-DataGrid-cellEditorOccluder {
  pointer-events: none;
  position: absolute;
  overflow: hidden;
}

.lm-DataGrid-cellEditorContainer {
  pointer-events: auto;
  position: absolute;
  background-color: #ffffff;
  box-sizing: border-box;
  box-shadow: 0px 0px 6px #006bf7;
  border: 2px solid #006bf7;
}

.lm-DataGrid-cellEditorContainer.invalid {
  box-shadow: 0px 0px 6px red;
  border: 2px solid red;
}

.lm-DataGrid-cellEditorContainer > form {
  width: 100%;
  height: 100%;
  overflow: hidden;
}

.lm-DataGrid-cellEditorWidget {
  width: 100%;
  height: 100%;
  outline: none;
  box-sizing: border-box;
}

.lm-DataGrid-cellEditorInput {
  background-color: #ffffff;
  border: 0;
}

.lm-DataGrid-cellEditorCheckbox {
  margin: 0;
}

.lm-DataGrid-notification {
  position: absolute;
  display: flex;
  overflow: visible;
  animation: fade-in 300ms ease-out;
}

.lm-DataGrid-notificationContainer {
  box-shadow: 0px 2px 5px #999999;
  border-radius: 3px;
  background-color: white;
  color: black;
  border: 1px solid black;
  font-family: sans-serif;
  font-size: 13px;
  padding: 4px;
}

@keyframes fade-in {
  0% {
    opacity: 0;
  }
  50% {
    opacity: 0.7;
  }
  100% {
    opacity: 1;
  }
}

[afshin]

If you compare the files:

wget https://registry.npmjs.org/@lumino/default-theme/-/default-theme-0.2.4.tgz
wget https://raw.githubusercontent.com/jupyterlab/lumino/master/packages/default-theme/style/datagrid.css
tar -xzf default-theme-0.2.4.tgz
diff package/style/datagrid.css datagrid.css

There is no difference.

[sopsick]

My apologies. I just double checked my repo and it was pulling and older version. Sorry about that.

[vidartf]

Do we still want to use this issue to track .invalid ?

[sopsick]

I would suggest opening a new one for the invalid issue so as not to confuse with the amateur mistake I made.