Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Insert style-loader loaded CSS at top of <head/> #382

Merged
merged 13 commits into from
Nov 21, 2018

Conversation

valentijnnieman
Copy link
Contributor

This fixes and closes #380 by inserting the default CSS loaded by style-loader at the top of <head/>, so that user loaded CSS will be below it, allowing users to overwrite the default CSS.

@chriddyp
Copy link
Member

Could we add an integration test with percy here? Something that mimics the user code by supplying some CSS in the assets/ folder that overrides a dash core component style.

@valentijnnieman
Copy link
Contributor Author

valentijnnieman commented Nov 15, 2018

@chriddyp Good point - I've added one. I tried to do some assertions on the CSS being set, but couldn't get it to work. The suggested value_of_css_property() returned incorrect values - rgb(0,0,0) for colors and 1184px for widths - no matter what it was.

Edit: Ok, seems the Percy snapshot isn't picking up the CSS changes either - even though running the test locally you can see them be set.

@chriddyp
Copy link
Member

Seems the Percy snapshot isn't picking up the CSS changes either

With percy, you need to explicitly set the CSS folder and CSS URL. From #312:

Percy works by downloading the HTML on the page, sending that HTML to their server, and then re-rendering that HTML in different browsers and taking screenshots.
Our CSS isn't actually on the page - it's referenced by a . Percy won't download these links, it needs us to supply their location on the filesystem (a mapping between the URL and the filenames).

See the percy Setup docs here: https://docs.percy.io/docs/python-selenium

You can also reach out to the percy team if you can't get those settings to work.

@valentijnnieman
Copy link
Contributor Author

@chriddyp Is it working in the main dash repo? I see that in Dash there's a test/assets folder, https://github.com/plotly/dash/tree/master/tests/assets but I can't figure out how that's working / what the set up is.

@chriddyp
Copy link
Member

Is it working in the main dash repo?

Not sure off the top of my head. You might be able to check out the Percy snapshots in that repo and see they look like they have custom css applied.

@chriddyp
Copy link
Member

Also, looking at git blame, @T4rk1n was the last one to work in the dash/tests/assets folder, so he can help answer your questions about that part of the code

@T4rk1n
Copy link
Contributor

T4rk1n commented Nov 19, 2018

Just tested and it doesn't load the assets in the dash tests percy snapshots.

@valentijnnieman
Copy link
Contributor Author

Just tested and it doesn't load the assets in the dash tests percy snapshots.

Ah too bad. Thanks for taking a look!

@T4rk1n
Copy link
Contributor

T4rk1n commented Nov 19, 2018

But I got them to load with this config:

loader = percy.ResourceLoader(
            webdriver=cls.driver,
            base_url='/assets',
            root_dir='tests/assets'
        )

https://percy.io/plotly/dash/builds/1182426?utm_campaign=plotly&utm_content=dash&utm_source=github_status

@valentijnnieman
Copy link
Contributor Author

@T4rk1n Anything else you've changed? I still can't get them to load.

@T4rk1n
Copy link
Contributor

T4rk1n commented Nov 19, 2018

No only thing I see different is the root_dir is relative 'test/assets'.

@T4rk1n
Copy link
Contributor

T4rk1n commented Nov 19, 2018

Here you can see the whole changes:

plotly/dash#461

@valentijnnieman
Copy link
Contributor Author

@T4rk1n That was it - removing the root_static_dir suggested by the Percy docs and having the root_dir relative fixed it. Thanks for your help!

@T4rk1n
Copy link
Contributor

T4rk1n commented Nov 19, 2018

Great, look like it worked 💃

You can review plotly/dash#461 while you are at it ?

@valentijnnieman valentijnnieman merged commit 27d2309 into master Nov 21, 2018
@valentijnnieman valentijnnieman deleted the hotfix-style-loader branch November 21, 2018 18:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update from 0.37.7 to 0.38.0 break local css for Dash components
3 participants