Skip to content
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

Making optionalZipCountries JS code conditional #2996

Merged
merged 15 commits into from
Mar 9, 2023

Conversation

loekvangool
Copy link
Contributor

@loekvangool loekvangool commented Jan 25, 2023

js/varien/form.js checks for existence of the variable, hence it's allowed to be undefined if empty.

There is more on this. The block is inserted into every Magento page, which seems excessive. It's probably only used in the Cart but I haven't checked thoroughly so leaving it alone for now. Same for the admin.

Also type="text/javascript" is no longer necessary.

`js/varien/form.js` checks for existence of the variable, hence it's allowed to be undefined if empty.

There is more on this. The block is inserted into every Magento page, which seems excessive. It's probably only used in the Cart but I haven't checked thoroughly so leaving it alone for now. Same for the admin.
@github-actions github-actions bot added Component: Directory Relates to Mage_Directory Template : base Relates to base template labels Jan 25, 2023
Copy link
Contributor

@luigifab luigifab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not, can you fix indent, use endif in template, and change Mage::helper with $this->helper? (not yet tested)
There is also the same file in adminhtml.

@loekvangool
Copy link
Contributor Author

I will never understand why templates are written in this horrible coding standard, lol.

…l_zip_countries.phtml

Co-authored-by: Fabrizio Balliano <fabrizio.balliano@gmail.com>
@kiatng
Copy link
Contributor

kiatng commented Feb 8, 2023

@sreichel installed phpmd and phpcs:
image

One of the rules is camelCase. Not against snake_case, but it is better to standardize on camelCase for everything, except in xml?

@loekvangool
Copy link
Contributor Author

good to go

@fballiano fballiano merged commit e61fbc2 into OpenMage:1.9.4.x Mar 9, 2023
fballiano added a commit that referenced this pull request Mar 9, 2023

---------

Co-authored-by: Fabrizio Balliano <fabrizio.balliano@gmail.com>
@fballiano
Copy link
Contributor

merged and 20ed

@loekvangool loekvangool deleted the patch-8 branch March 9, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Directory Relates to Mage_Directory Template : base Relates to base template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants