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

A few CSS fixes for regular-table in shadow DOM #2526

Merged
merged 7 commits into from
Feb 15, 2024

Conversation

tomjakubowski
Copy link
Contributor

@tomjakubowski tomjakubowski commented Feb 6, 2024

Moving datagrid's regular-table into shadow DOM caused some CSS regressions. These changes fix a few that we've spotted, particularly in the table header.

tomjakubowski and others added 4 commits February 5, 2024 13:06
With regular-table in the rule, the variables won't apply to the computed style lookups
used in datagrid because its regular-table is in the shadow DOM.

Co-authored-by: Ada Mandala <[email protected]>
perspective-viewer.dragging regular-table {
pointer-events: none;
perspective-viewer.dragging,
:host-context(perspective-viewer.dragging) {
Copy link
Contributor Author

@tomjakubowski tomjakubowski Feb 6, 2024

Choose a reason for hiding this comment

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

I could use some help with testing this rule. I couldn't find anywhere where dragging is set on the viewer's classes, and various drag/drop operations I tried didn't seem to add the class name to the viewer either

#efb92d,
#ed9c25,
#eb7e20,
#e75d1e,
Copy link
Contributor Author

@tomjakubowski tomjakubowski Feb 6, 2024

Choose a reason for hiding this comment

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

there were several formatting-only changes like this. I'm not sure how this happened - I have format with save enabled in VSCode, but it should be using the same prettier settings that the linter uses. And the linter accepts it in either style

@@ -64,7 +64,8 @@ perspective-string-column-style[theme="Monokai"] {
--rt-neg-cell--color: #ff6188 !important;
--rt--border-color: #444444;

// TODO: probably doesn't work with shadow DOM
// FIXME: broken in shadow DOM. Can be fixed with a new custom property allowing empty header cell backgrounds
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took a stab at this, but I couldn't work out how to declare the "default" value for the CSS custom property so that the theme override's was more specific than it

@texodus texodus added the bug Concrete, reproducible bugs label Feb 15, 2024
Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

Reviewed offline.

@texodus texodus merged commit 1489f0b into finos:master Feb 15, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Concrete, reproducible bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants