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

Inline spinner SVG on avatar change template #1748

Closed
wants to merge 1 commit into from

Conversation

arku
Copy link

@arku arku commented Jul 11, 2019

Fixes #1315

Also updates the alt text for the image.

@@ -2,7 +2,7 @@
<section class="modal-panel">
<div class="main-avatar">
<div id="loading-avatar-spinner">
<img alt="" src="../../../images/spinnerlight.svg">
<img alt="Loading..." src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz4KPHN2ZyB3aWR0aD0iNzNweCIgaGVpZ2h0PSI3M3B4IiB2aWV3Qm94PSIwIDAgNzMgNzMiIHZlcnNpb249IjEuMSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB4bWxuczp4bGluaz0iaHR0cDovL3d3dy53My5vcmcvMTk5OS94bGluayI+CiAgICA8IS0tIEdlbmVyYXRvcjogU2tldGNoIDQ2LjIgKDQ0NDk2KSAtIGh0dHA6Ly93d3cuYm9oZW1pYW5jb2RpbmcuY29tL3NrZXRjaCAtLT4KICAgIDxkZWZzPgogICAgICAgIDxsaW5lYXJHcmFkaWVudCB4MT0iOTMuMDkyODA5NiUiIHkxPSI1Mi43NzM0Mzc1JSIgeDI9IjY4LjUxMzMzOTglIiB5Mj0iMTE5LjMyNjAwNyUiIGlkPSJsaW5lYXJHcmFkaWVudC0xIj4KICAgICAgICAgICAgPHN0b3Agc3RvcC1jb2xvcj0iIzBBODRGRiIgc3RvcC1vcGFjaXR5PSIwIiBvZmZzZXQ9IjAlIj48L3N0b3A+CiAgICAgICAgICAgIDxzdG9wIHN0b3AtY29sb3I9IiMwQTg0RkYiIG9mZnNldD0iNjkuMzY5ODE4MiUiPjwvc3RvcD4KICAgICAgICAgICAgPHN0b3Agc3RvcC1jb2xvcj0iIzBBODRGRiIgb2Zmc2V0PSIxMDAlIj48L3N0b3A+CiAgICAgICAgICAgIDxzdG9wIHN0b3AtY29sb3I9IiMyNDg0QzYiIHN0b3Atb3BhY2l0eT0iMC4wMDQ3Nzc2Njk1MSIgb2Zmc2V0PSIxMDAlIj48L3N0b3A+CiAgICAgICAgICAgIDxzdG9wIHN0b3AtY29sb3I9IiMyNDg0QzYiIHN0b3Atb3BhY2l0eT0iMCIgb2Zmc2V0PSIxMDAlIj48L3N0b3A+CiAgICAgICAgICAgIDxzdG9wIHN0b3AtY29sb3I9IiMyNDg0QzYiIHN0b3Atb3BhY2l0eT0iMCIgb2Zmc2V0PSIxMDAlIj48L3N0b3A+CiAgICAgICAgPC9saW5lYXJHcmFkaWVudD4KICAgICAgICA8cmVjdCBpZD0icGF0aC0yIiB4PSIwIiB5PSIwIiB3aWR0aD0iNDgiIGhlaWdodD0iNjAiPjwvcmVjdD4KICAgIDwvZGVmcz4KICAgIDxnIGlkPSJQYWdlLTEiIHN0cm9rZT0ibm9uZSIgc3Ryb2tlLXdpZHRoPSIxIiBmaWxsPSJub25lIiBmaWxsLXJ1bGU9ImV2ZW5vZGQiPgogICAgICAgIDxnIGlkPSJTaGFwZSIgdHJhbnNmb3JtPSJ0cmFuc2xhdGUoLTUuMDAwMDAwLCAtMS4wMDAwMDApIj4KICAgICAgICAgICAgPHBhdGggZD0iTTQxLjgsNzMuOCBDMjEuOSw3My44IDUuOCw1Ny43IDUuOCwzNy44IEM1LjgsMTguMSAyMS42LDIuMiA0MS4xLDEuOCBDNDEuMywxLjggNDEuNCwxLjggNDEuNCwxLjggQzQxLjUsMS44IDQxLjcsMS44IDQxLjgsMS44IEM0NC42LDIuMiA0Ni44LDQuNSA0Ni44LDcuMyBDNDYuOCwxMC4xIDQ0LjYsMTIuNSA0MS44LDEyLjcgQzI4LDEyLjggMTYuOCwyNCAxNi44LDM3LjggQzE2LjgsNTEuNiAyOCw2Mi44IDQxLjgsNjIuOCBDNTUuNiw2Mi44IDY2LjgsNTEuNiA2Ni44LDM3LjggTDc3LjgsMzcuOCBDNzcuOCw1Ny43IDYxLjcsNzMuOCA0MS44LDczLjggWiIgZmlsbD0idXJsKCNsaW5lYXJHcmFkaWVudC0xKSI+PC9wYXRoPgogICAgICAgICAgICA8bWFzayBpZD0ibWFzay0zIiBmaWxsPSJ3aGl0ZSI+CiAgICAgICAgICAgICAgICA8dXNlIHhsaW5rOmhyZWY9IiNwYXRoLTIiPjwvdXNlPgogICAgICAgICAgICA8L21hc2s+CiAgICAgICAgICAgIDxnIGlkPSJNYXNrIj48L2c+CiAgICAgICAgICAgIDxwYXRoIGQ9Ik00MS44LDczLjggQzIxLjksNzMuOCA1LjgsNTcuNyA1LjgsMzcuOCBDNS44LDE4LjEgMjEuNiwyLjIgNDEuMSwxLjggQzQxLjMsMS44IDQxLjQsMS44IDQxLjQsMS44IEM0MS41LDEuOCA0MS43LDEuOCA0MS44LDEuOCBDNDQuNiwyLjIgNDYuOCw0LjUgNDYuOCw3LjMgQzQ2LjgsMTAuMSA0NC42LDEyLjUgNDEuOCwxMi43IEMyOCwxMi44IDE2LjgsMjQgMTYuOCwzNy44IEMxNi44LDUxLjYgMjgsNjIuOCA0MS44LDYyLjggQzU1LjYsNjIuOCA2Ni44LDUxLjYgNjYuOCwzNy44IEw3Ny44LDM3LjggQzc3LjgsNTcuNyA2MS43LDczLjggNDEuOCw3My44IFoiIGZpbGw9IiMwQTg0RkYiIG1hc2s9InVybCgjbWFzay0zKSI+PC9wYXRoPgogICAgICAgIDwvZz4KICAgIDwvZz4KPC9zdmc+Cg==">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if you want the Loading text. Is it at all useful? Also if we keep it then we should probably translate it too?

Copy link
Author

@arku arku Jul 11, 2019

Choose a reason for hiding this comment

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

Is it at all useful?

Of course it is, as it improves accessibility. AFAIK, screen readers use it.

Also if we keep it then we should probably translate it too?

You're right. Let me check how we are handing i18n in the codebase.

EDIT:

I am not sure if screen readers support all the languages we support. Btw, I looked for other image alt texts in the codebase and they are all in English.

How should we go ahead with this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if you want the Loading text. Is it at all useful?

Me either, however I don't think it will hurt. Lets wrap in translation block if keeping it.

@vbudhram vbudhram self-assigned this Aug 5, 2019
@vbudhram
Copy link
Contributor

vbudhram commented Aug 5, 2019

@arku Any updates?

@shane-tomlinson
Copy link
Contributor

Hi @arku, thank you for the patch, it would be a shame to let this bit rot and not be merged. Are you still interested in getting this landed? If so, let us know before Oct 1 whether you want to make the necessary updates. If we don't hear from you, we'll either close this or someone within the team will try to get it across the line.

@shane-tomlinson
Copy link
Contributor

Hi @arku - it looks like the original issue was fixed in 1723d0e and this PR is no longer needed. I'm sorry your contribution did not land into prod, we are always looking for new contributors!

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.

spinnerlight.svg is 404 on avatar_change
4 participants