Replace slack-self-invite.svg with icon-slack.svg on CoP page

[aadilahmed]

Fixes #5827

What changes did you make?

• Replaced img tag for the Slack icon underneath the Communities of Practice header with a liquid image element referencing _includes/svg/icon-slack.svg
• Modified CSS to maintain current appearance of page

Why did you make the changes (we will use this info to test)?

• To make sure that only a single slack icon svg is used in the codebase
• Make it easier to render the svg in various colors throughout the website

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Visuals after changes are applied

[github-actions]

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b aadilahmed-refactor-communities-of-practice-page-5827 gh-pages
git pull https://github.com/aadilahmed/website.git refactor-communities-of-practice-page-5827

---

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/aadilahmed/website/blob/refactor-communities-of-practice-page-5827/CONTRIBUTING.md  

[colin-macrae]

ETA: 1/24
Availability: 2 hrs on 1/23 and 2 hrs on 1/24

[agutiernc]

ETA: 1/22 - 7:20 pm
Availability: Evening 1/22

[agutiernc]

The following check out:

• correct linked issue and branch
• svg liquid tag replaced img tag
• CSS set for svg icon
• Changes are reflected in different browsers and viewports

Great job @aadilahmed ! Thanks for contributing.

[colin-macrae]

Hi @aadilahmed,

Thanks for taking this issue. It looks great.

The:

• to branch and from branch are correct
• code is clean and functions correctly in the browser
• linked issue is done correctly
• colors, margins, height and width look good in the browser (nice job adding the min-width property with a value of 24px)

Thanks for contributing!

[roslynwythe]

@aadilahmed @colin-macrae @agutiernc Thank your work on this PR. There have been reports of problems with support of svg styles on the Safari browser and so I hoped to do some testing in Safari, but I don't have an appropriate dev environment that currently works. Can I ask, were any of you able to check out the page in Safari? If not, would it be possible to perform that testing?

If testing does reveal problems in Safari, please see see this PR #5900 which addresses a issue similar to yours. One of the files changed in that PR is pages/getting-started.html and in that file, the developer created an svg element that wraps the included svg, and makes use of the 'viewBox' attribute of the outer svg element for scaling. There is also a "width" attribute on the outer svg element. Hopefully using that technique will provide cross-browser results for your issue as well.

[freaky4wrld]

@aadilahmed @colin-macrae @agutiernc Thank your work on this PR. There have been reports of problems with support of svg styles on the Safari browser and so I hoped to do some testing in Safari, but I don't have an appropriate dev environment that currently works. Can I ask, were any of you able to check out the page in Safari? If not, would it be possible to perform that testing?

If testing does reveal problems in Safari, please see see this PR #5900 which addresses a issue similar to yours. One of the files changed in that PR is pages/getting-started.html and in that file, the developer created an svg element that wraps the included svg, and makes use of the 'viewBox' attribute of the outer svg element for scaling. There is also a "width" attribute on the outer svg element. Hopefully using that technique will provide cross-browser results for your issue as well.

@roslynwythe @aadilahmed @colin-macrae @agutiernc the width of the slack image isn't rendered as intended i.e. 24px X 24px

Width Difference

Slack image width in the Hfla CoP page

Slack image width in the local environment CoP page

Suggested solution by @roslynwythe

You can go to the file and use #Layer1 element-id and make changes to achieve the intended behaviour.
Maybe @agutiernc can help @aadilahmed better as he implemented the changes in the above mentioned PR by @roslynwythe

Sorry, for being nosy I don't want to steal any GitHub credits from you guys....... just the issue caught my attention and I stop by as I have the privilege to the required browser

[agutiernc]

When looking at the slack icon in Safari, it renders the same as it does in another browser. However, in the update, the icon size is slightly smaller than the original no matter the browser used. It's not much of a difference, but the max-width, in the CSS, could probably be tweaked a bit to get a similar size.

Here's a screenshot of Safari displaying the original on the left side. On the right is the update that @aadilahmed did:

In PR #6098, I used an svg tag with some attributes to make sure that the icon renders on all browsers, including Safari. @aadilahmed, if you wish to do it that way, have a look at the code.

[roslynwythe]

Thank you @freaky4wrld - your comments are very useful. Since Safari is no longer supported on Windows, it is hard for many of us to do testing in Safari, and so your testing is really appreciated.

[aadilahmed]

The new slack icon svg has 2px of white space which the old svg did not. To match the size of the original icon, I increased the size of the svg to 26px by 26px, and adjusted the margins to align it in the same position.

Following the suggestion of @agutiernc I added outer svg tags around the liquid image element. I have no way of testing the website on Safari so I don't know if it works as intended on that browser.

[colin-macrae]

@aadilahmed @colin-macrae @agutiernc Thank your work on this PR. There have been reports of problems with support of svg styles on the Safari browser and so I hoped to do some testing in Safari, but I don't have an appropriate dev environment that currently works. Can I ask, were any of you able to check out the page in Safari? If not, would it be possible to perform that testing?

If testing does reveal problems in Safari, please see see this PR #5900 which addresses a issue similar to yours. One of the files changed in that PR is pages/getting-started.html and in that file, the developer created an svg element that wraps the included svg, and makes use of the 'viewBox' attribute of the outer svg element for scaling. There is also a "width" attribute on the outer svg element. Hopefully using that technique will provide cross-browser results for your issue as well.

Hi @roslynwythe . No, I do not have access to a Safari browser in which to test the changes. Should I remove myself as a reviewer?

And thank you @freaky4wrld, for helping on this.

[agutiernc]

Thanks for the revision @aadilahmed. I was able to verify the svg icon size in Safari. It displays the same as in Chrome. Here's a screenshot:

Appreciate your time for the update.

[agutiernc]

I approve the changes due to the svg icon being viewable in Safari and other browsers. Thanks for the update @aadilahmed

[freaky4wrld]

@aadilahmed thanks for taking the issue, I tested the changes in both safari and chrome and they work fine, the branches are correct, the issue is linked correctly. Thanks for taking time to make those changes..... just want to point out that the width, height are 24px in the issue, but you used 26px can you explain why and @roslynwythe can you check if the preferred value is valid.

[aadilahmed]

@aadilahmed thanks for taking the issue, I tested the changes in both safari and chrome and they work fine, the branches are correct, the issue is linked correctly. Thanks for taking time to make those changes..... just want to point out that the width, height are 24px in the issue, but you used 26px can you explain why and @roslynwythe can you check if the preferred value is valid.

The new slack icon svg has 2px of white space which the old svg did not. To match the size of the original icon, I increased the size of the svg to 26px by 26px, and adjusted the margins to align it in the same position.