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

Fixed adding new css class to previously added CSS class key for bug … #19700

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lawgiver00
Copy link

#19012

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues Fixes #19012, if you tried to add another class to an already existing key as described in the dumentation for addCssClass it would ignore any attempt except for the first one. This fix find the key and adds the additional classes as originally intended.

@what-the-diff
Copy link

what-the-diff bot commented Dec 1, 2022

  • Added a new css class to previously added CSS class key
  • Changed visibility of yii\web\View::isPageEnded to protected

framework/CHANGELOG.md Outdated Show resolved Hide resolved
framework/helpers/BaseHtml.php Outdated Show resolved Hide resolved
framework/helpers/BaseHtml.php Outdated Show resolved Hide resolved
@samdark samdark added the pr:request for unit tests Unit tests are needed. label Dec 1, 2022
@yii-bot
Copy link

yii-bot commented Dec 1, 2022

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests.

Could you add these please?

Thanks!

P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests

This is an automated comment, triggered by adding the label pr:request for unit tests.

@rob006
Copy link
Contributor

rob006 commented Dec 5, 2022

if you tried to add another class to an already existing key as described in the dumentation for addCssClass it would ignore any attempt except for the first one.

This is intended behavior, it is explained in addCssClass() documentation:

If class specification at given options is an array, and some class placed there with the named (string) key, overriding of such key will have no effect.

8c49ef0b

The only suspicious thing in #19012 is that behavior differs between string and array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:request for unit tests Unit tests are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in BaseHtml::addCssClass()
4 participants