Preconditions (*)
- Magento OS 2.3.4
- LS Central: no idea
- LS Omni: no idea
- LS eCommerce - Magento: 1.3.0
Steps to reproduce (*)
- Search for Magento frontend files which are being overwritten by this module
Expected result (*)
- Finding 0 or as little as possible files
Actual result (*)
- Finding many overwritten files
Discussion
Modules should try to avoid overwriting core Magento frontend files, this is usually being done in a custom theme build by an agency or a system integrator.
Having overwritten files in a 3rd party module means you need to keep them updated after every single new Magento release and keeping things backwards compatible will probably become very hard.
Examples
I took the time to compare 2 overwritten phtml files from your module, to figure out the reason why it's getting overwritten.
src/Customer/view/frontend/templates/form/forgotpassword.phtml, this overwrites magento/module-customer/view/frontend/templates/form/forgotpassword.phtml
Problems:
- Magento updated their phtml files with improved escaping, this was not applied to your overwritten files
- You've changed the formatting of the code, making it harder to see what you actually changed, please try to avoid changing the formatting of the code
- The only significant differences I can find after trying to bring your files in line with Magento's core files are:
--- vendor/magento/module-customer/view/frontend/templates/form/forgotpassword.phtml 2020-01-10 06:20:40.000000000 +0100
+++ vendor/lsretail/lsmag-two/src/Customer/view/frontend/templates/form/forgotpassword.phtml 2020-04-29 14:47:30.000000000 +0200
@@ -16,9 +13,9 @@
<fieldset class="fieldset" data-hasrequired="<?= $block->escapeHtmlAttr(__('* Required Fields')) ?>">
<div class="field note"><?= $block->escapeHtml(__('Please enter your email address below to receive a password reset link.')) ?></div>
<div class="field email required">
- <label for="email_address" class="label"><span><?= $block->escapeHtml(__('Email')) ?></span></label>
+ <label for="email_address" class="label"><span><?= $block->escapeHtml(__('Username/Email')) ?></span></label>
<div class="control">
- <input type="email" name="email" alt="email" id="email_address" class="input-text" value="<?= $block->escapeHtmlAttr($block->getEmailValue()) ?>" data-mage-init='{"mage/trim-input":{}}' data-validate="{required:true, 'validate-email':true}">
+ <input type="text" name="email" alt="email" id="email_address" class="input-text" value="<?= $block->escapeHtmlAttr($block->getEmailValue()) ?>" data-mage-init='{"mage/trim-input":{}}' data-validate="{required:true}">
</div>
</div>
<?= $block->getChildHtml('form_additional_info') ?>
- Change 1: you replaced
Email with Username/Email
- Change 2: you changed the type of the input field from
email to text and removed the validation for email input
Is there no other way to achieve this? Are these changes really necessary for this module?
src/Customer/view/frontend/templates/form/login.phtml, this overwrites magento/module-customer/view/frontend/templates/form/login.phtml
--- vendor/magento/module-customer/view/frontend/templates/form/login.phtml 2020-01-10 06:20:40.000000000 +0100
+++ vendor/lsretail/lsmag-two/src/Customer/view/frontend/templates/form/login.phtml 2020-04-29 14:56:56.000000000 +0200
@@ -20,9 +19,9 @@
<fieldset class="fieldset login" data-hasrequired="<?= $block->escapeHtmlAttr(__('* Required Fields')) ?>">
<div class="field note"><?= $block->escapeHtml(__('If you have an account, sign in with your email address.')) ?></div>
<div class="field email required">
- <label class="label" for="email"><span><?= $block->escapeHtml(__('Email')) ?></span></label>
+ <label class="label" for="email"><span><?= $block->escapeHtml(__('Username/Email')) ?></span></label>
<div class="control">
- <input name="login[username]" value="<?= $block->escapeHtmlAttr($block->getUsername()) ?>" <?php if ($block->isAutocompleteDisabled()) : ?> autocomplete="off"<?php endif; ?> id="email" type="email" class="input-text" title="<?= $block->escapeHtmlAttr(__('Email')) ?>" data-mage-init='{"mage/trim-input":{}}' data-validate="{required:true, 'validate-email':true}">
+ <input name="login[username]" value="<?= $block->escapeHtmlAttr($block->getUsername()) ?>" <?php if ($block->isAutocompleteDisabled()) : ?> autocomplete="off"<?php endif; ?> id="email" <?php echo $_isLSR ? 'type="text"' : 'type="email"' ?> class="input-text" title="<?= $block->escapeHtmlAttr(__('Email')) ?>" data-validate="<?php echo $_isLSR ? "{required:true}" : "{required:true, 'validate-email':true}" ?>">
</div>
</div>
<div class="field password required">We can see basically the same changes as in the forgotpassword.phtml file.
So anyway, my complaint is: are all of these overwrites really necessary? Is there no other way to achieve the same goal? This makes it very hard for agencies or system integrators to figure out why after your module, a bunch of things on the frontend start changing out of nowhere and makes it hard for us to apply Magento upgrades, since who is now responsible for updating overwritten phtml files in a shop? We or you?
Sorry for the lengthy text, and my examples are probably not that good, but there are most likely other parts in your module which can be improved in better ways other then overwriting core Magento frontend files.
Hope this makes sense somehow, thanks!
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent