replace Mage_Adminhtml_Block_Tax_Rate_Title_Fieldset - #5714
Draft
Hanmac wants to merge 2 commits into
Draft
Conversation
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the admin Tax Rate form “Tax Titles” section by replacing the custom Mage_Adminhtml_Block_Tax_Rate_Title_Fieldset element usage with a standard form fieldset and rendering the titles via Mage_Adminhtml_Block_Tax_Rate_Title as a Varien_Data_Form_Element_Renderer_Interface.
Changes:
- Update
Mage_Adminhtml_Block_Tax_Rate_Titleto implementVarien_Data_Form_Element_Renderer_Interfaceand render viatoHtml(). - Switch the Tax Rate form to use a normal fieldset and attach the titles renderer.
- Mark the legacy
Mage_Adminhtml_Block_Tax_Rate_Title_Fieldsetas deprecated and adjust unit-test PhpStorm metadata + PHPStan baseline accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/Traits/PhpStormMetaData/BlocksTrait.php |
Removes the unused tax_rate_title_fieldset alias/class from the PhpStorm metadata mapping lists. |
app/code/core/Mage/Adminhtml/Block/Tax/Rate/Title/Fieldset.php |
Marks the legacy fieldset element class as deprecated. |
app/code/core/Mage/Adminhtml/Block/Tax/Rate/Title.php |
Implements Varien_Data_Form_Element_Renderer_Interface so the block can render form elements. |
app/code/core/Mage/Adminhtml/Block/Tax/Rate/Form.php |
Replaces the custom fieldset-element insertion with a standard fieldset + element renderer approach. |
.phpstan.dist.baselines/method.nonObject.php |
Removes the baseline ignore related to the old setLegend() call path. |
Comment on lines
+61
to
+64
| public function render(Varien_Data_Form_Element_Abstract $element) | ||
| { | ||
| return $this->toHtml(); | ||
| } |
Comment on lines
+149
to
+151
| $form->addFieldset('tax_rate_title_fieldset', ['legend' => Mage::helper('tax')->__('Tax Titles')]) | ||
| ->addField('title', 'text', []) | ||
| ->setRenderer($renderer); |
Comment on lines
15
to
+17
| * @method string getLegend() | ||
| * @method $this setLegend(string $value) | ||
| * @deprecated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (*)
This replaces
Mage_Adminhtml_Block_Tax_Rate_Title_Fieldsetwith a normal Fieldsetand uses
Mage_Adminhtml_Block_Tax_Rate_TitleasVarien_Data_Form_Element_Renderer_Interface.Old
Mage_Adminhtml_Block_Tax_Rate_Title_Fieldsetmight be removed.Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)