search

ericthehacker / magento2-configscopehints

Magento2 module to label system config fields when they are overridden at more specific scope(s).
MIT License
23 stars 12 forks source link

Magento 2.1 compatibility #3

Closed mustdobetter closed 7 years ago

mustdobetter commented 8 years ago

There are issues with the display of the scope hint text on system config fields in Magento 2.1. Please see attached screenshot.

screen shot 2016-07-24 at 12 25 53
maderlock commented 8 years ago

I think we're going to have to sort this out ourselves, @mustdobetter.

erikhansen commented 8 years ago

If either if you are willing to submit a PR to fix this issue, I'm sure @ericthehacker would appreciate it.

erikhansen commented 7 years ago

@ericthehacker Do you have any plans to fix 2.1.x compatibility any time in the near future? I have an M2 project right now where this module would come in very handy.

Since I took care of upgrading this extension to address the last breaking change, I think it's your turn. ;)

ericthehacker commented 7 years ago

@erikhansen @mustdobetter @maderlock I'm sure this issue is due to moving to UI components in 2.1. The soonest I could probably get to fixing it would be Christmas week, when I have some time off.

Sorry for the lack of response -- I've been very busy lately. :-/

hostep commented 7 years ago

I took a quick look at this issue in case anyone is interested.

Currently the overridden data gets added to the html output using the getScopeLabel method. And this is how the Scope Label is being rendered in Magento 2.0.x: https://github.com/magento/magento2/blob/2.0/app/code/Magento/Config/Block/System/Config/Form/Field.php#L60 In Magento 2.1.x, this has changed to: https://github.com/magento/magento2/blob/2.1/app/code/Magento/Config/Block/System/Config/Form/Field.php#L51

So the big issue is that the generated html is getting inserted into a html element which isn't closed yet which makes the browsers freak out.

I think that the way to solve this would involve rewriting the \Magento\Config\Block\System\Config\Form\Field::render method and adding your html directly in there at the correct place. But I'm not sure about this...

In case this gets fixed, I think this deserves a big version jump to v3.0.0, since it will probably be backwards incompatible with Magento 2.0.x

paales commented 7 years ago

Cant for this to be fixed!

ericthehacker commented 7 years ago

I have a fix in progress and should have a release this week. Stay tuned ... 👍

ericthehacker commented 7 years ago

All

2.1.x support is added in #5 . I'm going to do some more testing tomorrow (and hopefully merge and tag as 3.0.0), but if anyone wants to check out the associated feature branch in the meantime and give me any feedback, I'm all ears.

In this feature branch, I moved the actual output of the hint, since there's not a good place to inject without having to do string replacement on HTML otherwise.

Additionally, I found the functionality is affected by a core bug in 2.1.3.

Both of these are outlined in the updated README: https://github.com/ericthehacker/magento2-configscopehints/blob/feature/m2.1-support/README.md .

ericthehacker commented 7 years ago

All I've merged #5, so this issue should (finally) be resolved. Thanks!