broken image minor changes #43
Conversation
index.html
Outdated
| @@ -268,9 +287,9 @@ <h6 class="black-text">Currently organized by: </h6> | |||
| <li> | |||
| <div class="row valign-wrapper"> | |||
| <div class="col s2"> | |||
| <a href="http://rob.ee/"> | |||
| <a href="http://rob.ee/"> | |||
index.html
Outdated
| <img src="https://avatars3.githubusercontent.com/u/13132899?v=3&s=460" alt="Avatar of Robert Axelsen" class="circle responsive-img"> | ||
| </a> | ||
| </a> |
index.html
Outdated
| var image=document.getElementById(id+'new'); | ||
| var old=document.getElementById(id); | ||
| sub="<img src='"+link+"' alt='background picture showing a view of Vienna' />"; | ||
| console.log(sub); |
There was a problem hiding this comment.
At least the "sub" variable with its code is necessary for showing the alternative picture, which is the very purpose of this brokenImage function.
Only "console.log(sub)" isn't necessary for the code to work.
getresolution.js
Outdated
| @@ -0,0 +1,8 @@ | |||
|
|
|||
| function getResolution(){ | |||
There was a problem hiding this comment.
Great initiative, Matthias! Like your increasing number of contributions ;)
What is the issue you're trying to fix here? Is this related to a issue? If not, please create an issue describing the details resulting in you creating this PR.
Two things regarding this function:
- Should this maybe be added to index.js instead of kept in a separate file?
- There is a typo in the return statement, meaning the variable gets never returned
There was a problem hiding this comment.
In case "https://images.unsplash.com/" is down , the brokenImage function gets executed and shows the picture from the repo.
I think it could be necessary to get the screen resolution. However I did not use this getResolution function at all. Probably might be useful if someone is viewing the page from a real small screen and the code makes use of it.
index.html
Outdated
| <script> | ||
| function brokenImage(link,id){ | ||
| //alert("broken image!"); | ||
| console.log(id+" image broken"); |
There was a problem hiding this comment.
Please remove, or is there a need to keep this in production for logging purposes?
There was a problem hiding this comment.
I used to keep this usually because it helps me in debugging the code later.
I'd like to know how you deal with those sort of things.
|
Thanks for looking at the pull request. |
|
@DDCreationStudios Followed instructions (after obviously misunderstanding them) here on GitHub and merged without meaning to. Just wanted to merge my changes to Matthias' branch. +1 from my side though, but wanted you to review first. Please revert if you disagree with anything in my merge. @MatthiasLiszt No linting tool. I just spotted it because GitHub review tool highlighted the indentation mistakes. I used Atom plugin to beautify the files one more time, so we have a consistent indentation style. Also moved your broken image function to index.js. All in all, a good PR though! Thanks a lot for the work you put into this 😄 👍 |
|
@robeerob ok don't worry. I will open a new PR since at least the Readme.md is looking weird :D |
added some useful functions for dealing with broken images , getting screen resolution and
changed my personal name to Matthias Liszt ...
took me longer than expected