-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[16.0][IMP] web_responsive: Change replace for less dangerous extension options #2899
[16.0][IMP] web_responsive: Change replace for less dangerous extension options #2899
Conversation
…ions Replacing all the template is a bit dangerous if any other module tries to extend that tamplate. So this changes make the same as it is done on before, but using the other options provided by xpath that are less dangerous.
Nice simplification ! |
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.
LGTM 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'm not having the same problem... @legalsylvain @carolinafernandez-tecnativa, can you confirm it? |
I'm working with Firefox. Have you tried there? |
And anyway, you can see in your sample that there's a bit of lateral scroll. Maybe you have selected a wider screen resolution than me. |
Okay, I could reproduce it, but the problem is not introduced on this PR, you can see the same problem on 16.0 branch runboat I will review it BTW |
OK, but please check on Firefox on my example, where the lateral scroll is wild. |
@pedrobaeza the problem was related with modules web_save_discard_button and web_theme_classic Set both as rebel modules |
could you elaborate the problem. I can try to fix web_theme_classic (or web_save_discard_button) when I have a time. |
@legalsylvain these problems are caused by that modules: #2899 (review) |
a7776c3
to
7816048
Compare
Well, I didn't develop and don't use web_save_discard_button, but if there a problem between web_theme_classic and web_responsive, I can try to take a look. |
Removed commit to set as rebel, because it does not avoid the installation of the modules |
I think that this PR can be merged, if you agree |
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.
Working without problems uninstalling both undesired modules:
/ocabot merge minor
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at c98010b. Thanks a lot for contributing to OCA. ❤️ |
Replacing all the template is a bit dangerous if any other module tries to extend that tamplate.
So this changes make the same as it is done on before, but using the other options provided by xpath that are less dangerous.
cc @Tecnativa TT50275
ping @pedrobaeza @carolinafernandez-tecnativa