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

Fix help deprecation #6335

Merged
merged 3 commits into from
Aug 30, 2020
Merged

Fix help deprecation #6335

merged 3 commits into from
Aug 30, 2020

Conversation

kirya-dev
Copy link
Contributor

Too many deprecation notices

I am targeting this branch, because receive too many deprecation notices and in upgrades was not actual instructions.

Closes #6319.
Addition for #6215.

Changelog

### Fixed
- Remove unnecessary span wrapper for field help. Merge styles with paragraph.
- Prevent too many deprecation notices.

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2020

Codecov Report

Merging #6335 into 3.x will increase coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #6335      +/-   ##
============================================
+ Coverage     77.23%   77.50%   +0.26%     
+ Complexity     2618     2603      -15     
============================================
  Files           140      142       +2     
  Lines          7779     7779              
============================================
+ Hits           6008     6029      +21     
+ Misses         1771     1750      -21     
Impacted Files Coverage Δ Complexity Δ
src/Admin/BaseFieldDescription.php 80.78% <100.00%> (-1.13%) 87.00 <0.00> (+1.00) ⬇️
src/Form/FormMapper.php 74.13% <100.00%> (ø) 49.00 <0.00> (ø)
src/Form/Type/Filter/DateType.php 0.00% <0.00%> (ø) 5.00% <0.00%> (-1.00%)
src/Form/Type/Filter/NumberType.php 0.00% <0.00%> (ø) 5.00% <0.00%> (-1.00%)
src/Form/Type/Filter/DefaultType.php 0.00% <0.00%> (ø) 4.00% <0.00%> (-1.00%)
src/Admin/Extension/LockExtension.php 100.00% <0.00%> (ø) 10.00% <0.00%> (ø%)
src/Form/Type/Filter/DateTimeType.php 0.00% <0.00%> (ø) 5.00% <0.00%> (-1.00%)
src/Form/Type/Filter/DateRangeType.php 0.00% <0.00%> (ø) 5.00% <0.00%> (-1.00%)
src/Resources/skeleton/Admin.tpl.php 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0740ee1...1f0f5a7. Read the comment docs.

@kirya-dev
Copy link
Contributor Author

kirya-dev commented Aug 27, 2020

Before:
image

After:
image

Also deprecation notices decreased on 51 messages (this count of edit fields on my form).
as bonus point removed extra spaces around field labels

Copy link
Contributor

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! There are parts I don't understand. If you are fixing many unrelated things, consider making more commits with explanations 🙏

"Fix help deprecation" does not help understanding what you are doing.

src/Form/FormMapper.php Show resolved Hide resolved
src/Resources/views/Form/form_admin_fields.html.twig Outdated Show resolved Hide resolved
greg0ire
greg0ire previously approved these changes Aug 29, 2020
@@ -40,7 +40,7 @@ You MUST use Symfony's [`help`](https://symfony.com/doc/4.4/reference/forms/type
Before:
```php
$formMapper
->add('field', null, [
->add('field', null, [], [
Copy link
Member

Choose a reason for hiding this comment

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

Good catch! can we add both examples?

Copy link
Contributor Author

@kirya-dev kirya-dev Aug 30, 2020

Choose a reason for hiding this comment

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

Do you mean addHelp & setHelps ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@franmomu franmomu Aug 31, 2020

Choose a reason for hiding this comment

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

Well I meant like:

->add('field_with_help_in_form_options', null, ['help' => '<p>...</p>'])
->add('field_with_help_in_field_description_options', null, [], ['help' => '<p>...</p>'])

The idea with the original example I wrote was showing that if the help message contains HTML, the user should add help_html, so maybe I should've added two separated examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe.. But in new example we showing two moments how stop throwing deprecating notices

@jordisala1991 jordisala1991 merged commit 63de836 into sonata-project:3.x Aug 30, 2020
@jordisala1991
Copy link
Member

Thank you @kirya-dev

/**
* NEXT_MAJOR: Remove this method.
*
* @deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0. Use Symfony Form "help" option instead.
Copy link
Member

Choose a reason for hiding this comment

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

The version where this method was deprecated is already released, so "3.74" should be used instead of "3.x".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render help
7 participants