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

Fix Blink rendering due to broken CSS #7

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Fix Blink rendering due to broken CSS #7

merged 1 commit into from
Sep 10, 2024

Conversation

dat-boris
Copy link

@dat-boris dat-boris commented Sep 10, 2024

Summary:

We (well, Walt) discovered an issue with Chrome rendering oddly in our Labeling project.
Turn out this affects any Blink rendered browser (Edge, Brave) - and one of the CSS style sheet
dm/css/main.css was not applied. This does not affect Firefox or Safari, which use different
rendering engine.

We have not deployed anything to Label Studio for the last 3 weeks.

Digging in a bit, it turns out that only "some" of the style from dm/css/main.css is not applied.
Binary searching the style, this is the culprit line:

// works
.dm-grid-view__cell {
    box-sizing: border-box;
    padding: 16px 16px 0 0
}

// invalid!
.dm-grid-view__cell:nth-child(var(--column-count)) {
    padding-right: 0
}

// fail
.dm-grid-view__cell-header {
    background: #f9f9f9;
    display: flex;
    justify-content: space-betwee;
    padding: 5px;
    width: 100%
}

Removing that CSS selector seems to have fix the rendering issue - CSS variable cannot be used in sub selector
and this breaks (only) blink renderer.

From this, it is easy to reference the main branch issue. Here is the main Label
Studio fix, which reference a break that happens 2 weeks ago:

NOTE: that this is part of the data manager setup which will not be overwritten by
our front-end deploy (this is a separate package).

Issue: https://khanacademy.atlassian.net/browse/DI-1565

Test Plan

Deploy on test and see rendering is fixed.

(see test project, screenshot ommited since this is an open repo)

@dat-boris dat-boris self-assigned this Sep 10, 2024
We (well, Walt [1]) discover an issue with Chrome rendering oddly in our Labeling project.
Turn out this affects any Blink based browser (Edge, Brave) - and one of the CSS style sheet
`dm/css/main.css` was not applied.  This does not affect Firefox or Safari, which use different
rendering engine.

We have not deployed anything to Label Studio for the last 3 weeks.

Digging in a bit, it turns out that only "some" of the style from `dm/css/main.css` is not applied.
Binary searching the style, this is the culprit line:

```
// works
.dm-grid-view__cell {
    box-sizing: border-box;
    padding: 16px 16px 0 0
}

// invalid!
.dm-grid-view__cell:nth-child(var(--column-count)) {
    padding-right: 0
}

// fail
.dm-grid-view__cell-header {
    background: #f9f9f9;
    display: flex;
    justify-content: space-betwee;
    padding: 5px;
    width: 100%
}
```

Removing that CSS selector seems to have fix the rendering issue!

From this, it is easy to reference the main branch issue.  Here is the main Label
Studio fix, which reference a break that happens 2 weeks ago:

* issues (fixed in 1.13): HumanSignal#6256
* fix commit: HumanSignal@cb6e4c3

NOTE: that this is part of the `data manager` setup which will not be overwritten by
our front-end deploy (this is a seperate package).

Issue: https://khanacademy.atlassian.net/browse/DI-1565

Test Plan:

Deploy on test and see rendering is fixed.
Copy link

@wwells wwells 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 solving this so fast!

@dat-boris dat-boris merged commit 6f7942b into develop Sep 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants