Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[WebProfilerBundle] Render the toolbar stylesheet#58287

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:7.2fromsmnandre:dx/wdt-toolbar-stylesheet
Oct 10, 2024

Conversation

smnandre
Copy link
Member

QA
Branch?7.2
Bug fix?no
New feature?no
Deprecations?no
IssuesFix #...
LicenseMIT

Render the (static) toolbar stylesheet separately from the (dynamic) toolbar content.

(avoid the 20ko inlined CSS injection on every page)

LauLaman, MatTheCat, vinceAmstoutz, Kocal, and mtarld reacted with thumbs up emojiju1ius reacted with thumbs down emoji
@javiereguiluzjaviereguiluz added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 7, 2024
Copy link
Contributor

@tucksauntucksaun left a comment

Choose a reason for hiding this comment

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

nice one 👍

@fabpotfabpotforce-pushed thedx/wdt-toolbar-stylesheet branch from7c9dd5a toc36fcffCompareOctober 10, 2024 05:34
@fabpot
Copy link
Member

Thank you@smnandre.

@fabpotfabpot merged commitd83167d intosymfony:7.2Oct 10, 2024
3 of 4 checks passed
@fabpotfabpot mentioned this pull requestOct 27, 2024
@alexislefebvre
Copy link
Contributor

alexislefebvre commentedDec 2, 2024•
edited
Loading

This change breaks the style of the toolbar for some users:

What about:

1.adding a parameter to use the “old” way?
2. reverting the changes from this PR to get back to the “old” way, and adding an opt-in parameter to use the “new” way?

Update: never mind, a solution has been found:

fabpot added a commit that referenced this pull requestJan 6, 2025
…xislefebvre)This PR was merged into the 7.2 branch.Discussion----------[WebProfilerBundle] fix loading of toolbar stylesheet| Q             | A| ------------- | ---| Branch?       | 7.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        |Fix#59045| License       | MITIt looks like this PR-#58287Caused issues with some configurations:-#59045According to the thumb-up emoji on [this comment](#59045 (comment)) (I don’t have a better measurement of the impact), it affected at least 10 users, with various web servers.Proposals:1. do not use the `.css` file extension so that servers do not try to serve an actual file2. if we consider that the disappearance of the style of the profiler’s toolbar is a breaking change, the `.css` file extension could be added back with Symfony 8.0, with a note to help people upgrade (see the workarounds in the issue)Commits-------7fef930 fix: loading of WebProfilerBundle’s toolbar stylesheet
@ju1ius
Copy link
Contributor

Render the (static) toolbar stylesheet separately from the (dynamic) toolbar content.

If the stylesheet is static why not just serve a static asset instead ?

@smnandre
Copy link
MemberAuthor

I guess because service a static asset would require to know its public path, which is harder to grant than a routed URL ?

ju1ius reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@rosierrosierrosier left review comments

@tucksauntucksauntucksaun left review comments

@fabpotfabpotfabpot approved these changes

@stofstofAwaiting requested review from stof

@OskarStarkOskarStarkAwaiting requested review from OskarStark

Assignees
No one assigned
Labels
❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: ReviewedWebProfilerBundle
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.

10 participants
@smnandre@fabpot@alexislefebvre@ju1ius@rosier@stof@tucksaun@OskarStark@javiereguiluz@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp