Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

EZP-31788: Remove duplicate icon from svg file #1444

Merged
merged 1 commit into from
Aug 7, 2020
Merged

EZP-31788: Remove duplicate icon from svg file #1444

merged 1 commit into from
Aug 7, 2020

Conversation

ITernovtsii
Copy link
Contributor

Question Answer
Tickets EZP-31788
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

There are duplicate "portfolio" icons in ez-icons.svg, removing the oldest one.

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@adamwojs adamwojs changed the title remove duplicate icon from svg file EZP-31788: Remove duplicate icon from svg file Aug 6, 2020
@adamwojs
Copy link
Member

adamwojs commented Aug 6, 2020

Indeed portfolio id is duplicated but I will leave decision to frontend team 😉

@dew326
Copy link
Member

dew326 commented Aug 6, 2020

I see this also affects the 2.5. Could you rebase to 1.5 branch?

@ITernovtsii ITernovtsii changed the base branch from 2.0 to 1.5 August 6, 2020 09:47
@ITernovtsii
Copy link
Contributor Author

Rebased to 1.5

Duplicate IDs lead to icons not displayed in chrome.
I can reproduce it only on edit page, and only when chrome loading file from disk cache, and only if page opened from a non-admin domain.
Screenshot from 2020-08-06 12-34-31

I manually replaced file to the one I have in PR, and can't reproduce chrome issue anymore.
After I replaced it back to "broken" icons file, I can reproduce it again.
So, I'm sure it's related to "portfolio" icon duplicated.

@dew326
Copy link
Member

dew326 commented Aug 6, 2020

Thanks for sharing the details of the issue.
We would merge it anyway, but I only wanted to merge it with the proper version.

We have spotted the issue with missing icons in some rare cases but didn't investigate it more yet. I think you saved us a lot of time debugging this. Thanks a lot.

@adamwojs adamwojs merged commit 96af6b0 into ezsystems:1.5 Aug 7, 2020
@adamwojs
Copy link
Member

adamwojs commented Aug 7, 2020

Thank you for contribution @ITernovtsiy!

# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

5 participants