-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Unify css files and move vendors assets to assets folder #7084
Unify css files and move vendors assets to assets folder #7084
Conversation
Could you please rebase your PR and fix merge conflicts? |
5c88147
to
9a29f93
Compare
9a29f93
to
1859f80
Compare
Let's wait a little bit, have to test this one further |
Do you use a SonataAdmin project to test this PR (and the previous) in order to check the css still works ? |
I was testing the previous PR, but with this one I discovered an unrelated bug #7085 . Also with this PR some styles broke, so probably I am missing something. Just want to make sure the styles looks the same. |
e52e535
to
723dfdb
Compare
Resolved the problems, my mistake importing files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you prefer to fix #7087 before a merge ?
There are two issues there: the first one I dont know yet how to fix, have to take a look, the second one is because I need to use the full version of select2. Anyway both issues are related to select2 js. Not related to this Pr. |
723dfdb
to
f47f526
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need an upgrade note for these changes?
Regarding the changelog messages:
About the term "vendor", I think we must remove the backticks if we are using it in a colloquial way, otherwise we should specify if we are referencing a directory name or something else.
About the term "folder", my advice is to replace it with "dir" or "directory", since the folder concept is just a graphical representation for the real filesystem structure, called directory.
👍 applied the changes to the changelog. About the upgrade note, I think we wrote one on the first PR warning about the fact that we are restricting what we show on the public folder. I think still applies the same. wdyt? |
If the upgrade notes for these changes are already covered in a previous PR, it's fine for me. |
Thanks @jordisala1991 |
Subject
I am targeting this branch, because this is BC break.
Part of #7049
Requires #7081
Changelog