-
Notifications
You must be signed in to change notification settings - Fork 1.7k
magento/devdocs#: Extends information about creating a dynamic row system config. #5992
magento/devdocs#: Extends information about creating a dynamic row system config. #5992
Conversation
An admin must run tests on this PR before it can be merged. |
a699e5c
to
f2a57dd
Compare
* | ||
* @return string | ||
*/ | ||
public function _toHtml(): string |
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.
It's not a big deal, but should we override the parent method scope of _toHtml()
? It's protected
in the parent class.
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.
@rogyar , I see such this approach in the existing Magento classes:
- Magento\CatalogInventory\Block\Adminhtml\Form\Field\Customergroup::_toHtml()
- Magento\Braintree\Block\Adminhtml\Form\Field\Countries::_toHtml()
- Magento\Braintree\Block\Adminhtml\Form\Field\CcTypes::_toHtml()
So, in my PR I've followed the same logic.
Thanks.
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.
I believe it's a sort of mechanical mistake. But, not a big deal, as I said.
running tests |
Hi @atwixfirster, thank you for your contribution! |
Purpose of this pull request
This pull request (PR) extends current Creating a dynamic row system config topic and adds information how to add a custom drop-down option.
Affected DevDocs pages
whatsnew
Added additional steps and code to the Creating a dynamic row system config section in the Extensions Best Practices Guide.