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

Dark mode breaks Tailwind styling in some situations since NiceGUI 2.0 #3753

Open
petergaultney opened this issue Sep 19, 2024 · 19 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@petergaultney
Copy link
Contributor

petergaultney commented Sep 19, 2024

Description

I'm not (currently) able to give a minimal reproducible example. In lieu of that, I'm going to write down the basic things I'm observing here, and then go spend some time debugging the NiceGUI code to see if I can figure out what's going on. I'll report back with any findings.

We upgraded from NiceGUI 1.4.30 to 2.1.0, and a lot of our CSS styling broke. We think it mostly has to do with tailwind classes somehow no longer being present/applied. I have narrowed it down to the 1.4.37 -> 2.0.0 upgrade by installing those versions specifically.

Systems affected are various, including multiple developer Mac laptops and our Linux-based deployment.

Screenshots, before and after the upgrade:

(expected layout, which we get in 1.4.37 and earlier)
Screenshot 2024-09-19 at 12 42 57

(what we see in 2.0)
Screenshot 2024-09-19 at 12 42 04

The relevant code, which was unchanged:

def _render_task_counts_display(task_counts: ty.Collection[QueueCount]) -> None:
    with ui.row(wrap=False).classes("w-full"):
        with ui.column().classes("w-1/2"):
            viz.render_task_counts_by_intent(task_counts)
            viz.render_task_counts_by_typename(task_counts)
        with ui.column().classes("w-1/2"):
            viz.render_task_counts_by_typename_and_intent(task_counts)


async def queue_stats_area():
    task_counts = await list_queue_counts("task")
    policy_counts = await list_queue_counts("policy")

    with ui.column().classes("w-full"):
        viz.render_total_task_count(task_counts)
        with ui.row(wrap=False).classes("w-full"):
            with ui.column().classes("w-1/2"):
                _render_task_counts_display(task_counts)
            with ui.column().classes("w-1/2"):
                viz.render_policy_counts(policy_counts)
@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 19, 2024

It's not just ui.column - i'm seeing this with ui.button and giving it a tailwind class like so:

    with ui.row().style("margin-top: 2rem; "):
        ui.button(
            "Reject Selected",
            on_click=lambda: render_reject_dialog(
                render_labels=render_labels, table=_labels_table_loaded.element
            ),
            color="orange-500",
        )

works in Nicegui 1.4.37:

Screenshot 2024-09-19 at 12 53 21

does not in 2.0:

Screenshot 2024-09-19 at 12 51 55

html

The NiceGUI code is producing exactly the same HTML in both cases (i have checked with diff):

1.4.37:

<button class="q-btn q-btn-item non-selectable no-outline q-btn--standard q-btn--rectangle q-btn--actionable q-focusable q-hoverable bg-orange-500" tabindex="0" type="button" id="c40"><span class="q-focus-helper" tabindex="-1"></span><span class="q-btn__content text-center col items-center q-anchor--skip justify-center row"><span class="block">Reject Selected</span></span></button>

2.1.0:

<button class="q-btn q-btn-item non-selectable no-outline q-btn--standard q-btn--rectangle q-btn--actionable q-focusable q-hoverable bg-orange-500" tabindex="0" type="button" id="c40"><span class="q-focus-helper" tabindex="-1"></span><span class="q-btn__content text-center col items-center q-anchor--skip justify-center row"><span class="block">Reject Selected</span></span></button>

css

I see tailwindcss.min.js under _nicegui<ver>/static in both installations. It's there. And I can tell there's a big difference between the two - far bigger than is possible for me to meaningfully analyze. But what's interesting is that the browser just doesn't seem to be doing anything with the bg-orange-500 class in the one case.

In 1.4.37, I see this computed style:

Screenshot 2024-09-19 at 13 18 27

But there are no similar computed styles in 2.1.

Further evidence that tailwind itself is at least notionally present is that I see it embedded as a <style> inside <head> on both versions:

1.4.37:
Screenshot 2024-09-19 at 13 24 18

2.1.0:
Screenshot 2024-09-19 at 13 24 57

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 19, 2024

aha - so, interestingly, the <head><style> tag for tailwind on 1.4.37 contains the following clear reference to the assigned background color:

Screenshot 2024-09-19 at 13 27 47
.bg-orange-500{--tw-bg-opacity:1;background-color:rgb(249 115 22 / var(--tw-bg-opacity))}

Whereas in 2.1, that (and several other things) do not appear:

Screenshot 2024-09-19 at 13 29 09

I don't know enough about how tailwind works. I am guessing that it is dynamically populating this <style> tag based on the styles "actually used" - and I suppose what I don't know is "when does it do that, and how?". Maybe we're not sending some kind of event that it relies on to dynamically update that section?

I can at least confirm that if I manually populate the 2.1 <style> tag at the end with the missing background color style, the browser instantly fixes the coloring of the button.

Screenshot 2024-09-19 at 13 32 17

@petergaultney
Copy link
Contributor Author

further updates:

I still don't really understand how Tailwind does this, but I can confirm that the <style> tag is being dynamically updated when the HTML changes. In fact, when I change the HTML directly in the browser (dev-tools/inspector), I find (in both versions of NiceGUI) that a CSS class is generated and automagically added to the end of that <style> tag.

However, weirdly enough, in NiceGUI 2.1, if I change bg-orange-500 to bg-red-500, the CSS is generated and added and the button immediately becomes red. But if I change it back to bg-orange-500, no such change happens - the CSS is not properly generated and the button inherits its transparent background. It's as if the code is running but is perhaps caching bad values?

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 19, 2024

here's a minimal reproduction. the plot thickens....

import sys

from nicegui import __version__, ui

ui.label(__version__)
ui.button("i'm the default color but i shouldn't be").tailwind.background_color("purple-500")
# the above seems like a bug in all versions to me

ui.button("i'm purple", color="orange-400").tailwind.background_color("purple-400")
ui.button("i'm orange", color="orange-500")

ui.run(port=int(sys.argv[1]))
Screenshot 2024-09-19 at 14 57 39 Screenshot 2024-09-19 at 15 00 26

It seems like I can't get exactly the same thing to happen in my minimal reproduction. So now I'm going to have to figure out... where is this bug somehow (presumably) being triggered in our own code, such that the exact same application has different behaviors with just a change to the nicegui install, but a different application on the same machine (and in fact running inside the same virtual envs) does not trigger the broken behavior? :/

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 20, 2024

further indications that there is a bug of some sort in NiceGUI or its dependencies, though I'm beginning to believe it's somewhat non-deterministic and possibly a race condition, is that if I run a background task with an asyncio.sleep, I get the color that I want:

        btn = ui.button(
            "Reject Selected",
            on_click=lambda: render_reject_dialog(
                render_labels=render_labels, table=_labels_table_loaded.element
            ),
            color="red-500",  # doesn't work
        )
        btn.tailwind.background_color("orange-400")  # doesn't work
        import asyncio

        await asyncio.sleep(2)
        btn.tailwind.background_color("yellow-400")  # doesn't work

        async def fixco():

            await asyncio.sleep(4)

            btn.tailwind.background_color("purple-500")  # works

        background_tasks.create(fixco())

resulting HTML (all classes present and in order):
Screenshot 2024-09-19 at 23 19 42

resulting CSS (only bg-purple-500 is ever added to the Tailwind CSS):
Screenshot 2024-09-19 at 23 20 16

@petergaultney
Copy link
Contributor Author

It seems plausible that it might be a bug in Vue? I don't know what is responsible for asking Tailwind to actually update the CSS. I don't see anything in NiceGUI that looks like it's involved.

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 20, 2024

final note for the evening: I do see that the docs call out some specific Quasar+Tailwind incompatibilities with Button. I have confirmed that if there's a Quasar class applied (e.g. bg-primary) then the Tailwind classes never apply.

But I have been seeing this behavior with the Tailwind classes w-full, w-1/2, etc. as well. And delayed application of the classes seems to make a difference in both cases. From what I can tell, the 'issue' here is that in newer versions of NiceGUI, it is somehow 'harder' to get the Tailwind styles to be actually defined in the page CSS under the <style> tag.

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 20, 2024

This morning, I think this must all be happening via the Tailwind CDN - NiceGUI is embedding Tailwind directly, and so it runs without a build/compile-time step. And yet, somehow, this new version of Tailwind is less consistent in NiceGUI 2.x than it was in 1.x - it misses "more" events than it used to, leading to missing CSS and broken pages.

I can readily imagine this might be something that NiceGUI itself has very little control over. And yet, apparently the newer version of Tailwind is somehow worse than what was being packaged before. I am going to start looking for workarounds and I'll report back if I come up with anything.

@petergaultney
Copy link
Contributor Author

I've confirmed by setting a breakpoint on the <style> tag managed by Tailwind that it is in fact tailwindcss.min.js that is mutating that style tag when the HTML changes. They use a MutationObserver to pick up on changes to classes and run their code.

Is there any chance that this has something to do with the order in which the Tailwind CDN/JS loads, as compared to the rest of the NiceGUI application? I wonder if they're simply not getting their MutationObserver registered early enough to catch the initial HTML being emitted by NiceGUI?

@petergaultney
Copy link
Contributor Author

Okay, I can confirm that simply by swapping out the newer version of Tailwind (3.4.5) for the older one from NiceGUI 1.4.x (3.3.2), the bad behavior goes away. I did this by copying nicegui/static/tailwindcss.min.js from the old install into the new install, a restart of the application, and finally, a hard refresh of the browser. I can confirm by looking at their source code that it's 3.3.2 running inside nicegui 2.1.

My width and color classes are now being applied correctly in my more complex application.

I am going to write up an issue for Tailwind, because what I'm seeing by stepping through the minified JS is that they've got a Set of known classes that already contains the classes that NiceGUI put into the HTML, but the corresponding CSS is not generated. Hence the odd behavior around how I can add new classes, see those styles generated, but then reverting back to the old HTML classes does not properly generate the previously missing classes. Once missed the first time, they are never generated by Tailwind even though the MutationObserver fires and they detect that the classes have been added to the HTML.

I might play around with even newer versions of Tailwind to see if any of them fix the issue. I somewhat doubt it, but you never know. :)

@petergaultney
Copy link
Contributor Author

Maybe you can help me figure out... it seems like the Tailwind 3.3.2 packaged by NiceGUI 1.4.x is not the same as what you can download from their CDN. And in fact any version that I download from their CDN exhibits the same 'broken' behavior. So I can't exactly post an issue over there yet, because I'm not sure what versions I'm actually dealing with.

@petergaultney
Copy link
Contributor Author

petergaultney commented Sep 20, 2024

here's my post over there. I'm struggling to wrap my head around their behavior, which seems 'wrong', but also how y'all could have such a different-looking minified Tailwind JS file compared to what their CDN offers for 3.3.2. Maybe they've just changed how they're minifying it?

@falkoschindler
Copy link
Contributor

Hi @petergaultney,

Thanks for the thorough investigation! What a journey!

By upgrading from TailwindCSS 3.2.0 to 3.4.10 in NiceGUI 2.0.0 we expected some minor hiccups, but not such fundamental styling issues. Even though they added some new classes, existing ones shouldn't break.

Unfortunately, I can't reproduce the problem. A button like ui.button("Reject Selected", color="orange-500") is shown orange as expected.

But I remember some trouble when upgrading the Tailwind dependency in PR #3654. It introduced layout problems, e.g. related to the resizing behavior of ui.echart, which could be fixed by changing the way the element waits for Tailwind classes to be applied:
https://github.com/zauberzeug/nicegui/pull/3654/files#diff-cea8c98fcb9226ca4b8e86974cbb09e00807012ca1278607a99464e92d0b9937

This might be related to your problem, since it is also a kind of timing issue.

Another thing I remember: While debugging the EChart layouting problem, I tried switching between Tailwind versions and noticed that I couldn't reproduce the Tailwind version included in NiceGUI 1.4.x - exactly like you're reporting. It's still unclear to me why the minified files differ. Maybe Tailwind re-packaged (and re-uploaded!?) the files for some reason, or we got it from another source when releasing NiceGUI 1.4.0 in October 2023. But I couldn't find any information about it.

So I hope we can find a reproduction and maybe a way to improve the reliability again. And maybe the Tailwind community can help.

@falkoschindler falkoschindler added help wanted Extra attention is needed bug Something isn't working labels Sep 25, 2024
@rodja
Copy link
Member

rodja commented Sep 26, 2024

Unfortunately, I can't reproduce the problem. A button like ui.button("Reject Selected", color="orange-500") is shown orange as expected.

@petergaultney could you try to create a minimal reproducible example which breaks the colors?

@petergaultney
Copy link
Contributor Author

Yeah, right now I simply can't reproduce a minimal reproduction for colors. But Tailwind is essentially broken everywhere in our app, unless I revert to the old version that was specifically packaged with NiceGUI 1.4.x.

One of our developers has further noted that by loading the parts of our UI that use Tailwind asynchronously (i.e. having them behind a nested async def function), Tailwind "starts working again" - which further suggests that this is some kind of startup race condition. I wonder if there is some sort of "startup period" for Tailwind after which it will react more correctly to new classes being introduced, and maybe the newer minified versions of Tailwind are somehow slower at startup? I feel fairly confident that it's not an issue of Tailwind itself being running, because when I add breakpoints and step through their code, I see Tailwind reacting to our addition of classes - it just doesn't 'return' all of the classes it sees in the rendered CSS the first few times.

I wish I were seeing any sort of engagement with my question over on Tailwind's page. One of their developers would likely have an immediate guess about what's happening, but so far I've not managed to trace their code down far enough to make sense of it (and I can't find the relevant original source code in order to unminify it either, which makes for some fairly painful reverse-engineering).

@rodja
Copy link
Member

rodja commented Sep 27, 2024

Sometimes it's really hard to find a minimal reproduction. You could try to remove code from your productive system until the error disappears. This takes time because the removal steps between restarts and checks should not be to big but will bring you closer to a situation where the error still happens.

@jdoiro3
Copy link

jdoiro3 commented Sep 27, 2024

After debugging this myself I found a minimal reproducible example. It looks to be a weird case where we set dark=None when calling ui.run, and nicegui sets the Quasar dark plugin to auto and does not add any tailwind config (see here), but in another place we use the ui.dark_mode element used in a toggle which sets the value to either True or False (never None/undefined). This plugin updates the tailwind config but for some reason this does not trigger the tailwind JIT style generation to run again. Not sure what the issue is but it's definitely related to these to values being different. I think the fix for us is to remove dark=None in ui.run since it's handled by another component in our app.

Note that the only case where the tailwind styling is not applied/generated is when ui.run(dark=None) and ui.dark_mode(True | False).

from nicegui import ui


def render_total_count(total: int) -> None:
    with ui.card().classes("w-full v-26 items-center").props("bordered"):
        ui.label("Total Count").classes("text-3xl")
        ui.separator()
        ui.label(str(total)).classes("text-2xl")


@ui.page("/")
def index() -> None:
    dark_mode = ui.dark_mode(True)
    with ui.column().classes("w-full"):
        render_total_count(50)


if __name__ in {"__main__", "__mp_main__"}:
    ui.run(
        dark=None,
    )

@falkoschindler
Copy link
Contributor

Oh wow, thanks @jdoiro3! Combining dark=None with ui.dark_mode(True) seems to break Taildwind completely!

from nicegui import ui

ui.dark_mode(True)
ui.label('This should be bordered').classes('border')

ui.run(dark=None)

With ui.dark_mode(False) or without dark=None the border is there.

@falkoschindler
Copy link
Contributor

Apparently there have been changes to Tailwind's dark mode configuration in 3.3 and 3.4:
https://tailwindcss.com/docs/dark-mode#toggling-dark-mode-manually
The documentation suggests to use the "selector" strategy instead of "class" and to add the "dark" class to document.html instead of document.documentElement. But whatever I try, something's always broken.

To verify dark mode, we need to make sure every ui.run(dark=...) value works nicely standalone or with different values in ui.dark_mode. Tailwind styling shouldn't break (of course) and the "dark" prefix needs to work, e.g. .classes(dark:bg-red-400).

At the moment it looks like we might need to re-implement the whole dark mode integration, maybe based on window.matchMedia('(prefers-color-scheme: dark)'). As a quick fix we could raise an exception when using ui.dark_mode while dark=None in ui.run.

@falkoschindler falkoschindler added this to the 2.x milestone Oct 4, 2024
@mattloose mattloose mentioned this issue Oct 6, 2024
3 tasks
@falkoschindler falkoschindler changed the title nicegui 2.0 seems to change/break some tailwind behavior? Dark mode breaks Tailwind styling in some situations since NiceGUI 2.0 Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants