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

[4.2] Clean the logo URL in Cassiopeia #39574

Merged
merged 5 commits into from
Jan 10, 2023

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue #39571, #39538, #39543

Summary of Changes

Use the recommended way to render an image: through the HTMLHelper

Testing Instructions

Check that selecting a custom logo image works and that the source doesn't have the hash part of the URL. Also check that svg renders as expected

Actual result BEFORE applying this Pull Request

The URL contains the hash (it is dirty) and SVG images get 0 width and height thus are not visible

Expected result AFTER applying this Pull Request

The URL does not contain the hash (it is clean) and SVG images are visible

Link to documentations

This is a bug fix without changing any of the existing functionality
Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@Quy
Copy link
Contributor

Quy commented Jan 9, 2023

Double //. Also image does not display on the frontend.

39574

@dgrammatiko
Copy link
Contributor Author

@Quy should be ok now, thanks

@Quy
Copy link
Contributor

Quy commented Jan 9, 2023

The double slashes is fixed, however, the image won't display even though the markup is there. Adding a width attribute displays the image.

<img loading="eager" decoding="async" src="http://localhost/joomla-cms-4.2-dev/images/logo.svg" alt="J42">

@dgrammatiko
Copy link
Contributor Author

however, the image won't display even though the markup is there. Adding a width attribute displays the image.

Ok, this could be solved either with CSS (patch) or the media manager needs to pass the correct size when an svg is selected (proper solution). I would make a PR for the second option.
This PR is still valid as it cleans the URL #...

@Quy
Copy link
Contributor

Quy commented Jan 9, 2023

I have tested this item ✅ successfully on cca0b50


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39574.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on cca0b50


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39574.

@chmst chmst changed the title [4.2] Clean the logo URL in Cassiopeia [4.2] Clean the logo URL in Cassiopeia Jan 10, 2023
@chmst
Copy link
Contributor

chmst commented Jan 10, 2023

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39574.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 10, 2023
@dgrammatiko
Copy link
Contributor Author

@Quy @viocassel thanks for testing this one. #39586 is the last missing part (the one I wrote about here: #39574 (comment)) and would be awesome if you could test that one as well. Mille grazie

@roland-d roland-d merged commit a348f05 into joomla:4.2-dev Jan 10, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 10, 2023
@roland-d
Copy link
Contributor

Thank you

@roland-d roland-d added this to the Joomla! 4.2.7 milestone Jan 10, 2023
@dgrammatiko dgrammatiko deleted the 4.2-dev-cassiopeia-logo branch January 10, 2023 19:55
charvimehradu pushed a commit to charvimehradu/joomla-cms that referenced this pull request Jan 26, 2023
* Clean the logo URL

* remove debugging

* fixit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants