How can i make it better? | Sololearn: Learn to code for FREE!
New course! Every coder should learn Generative AI!
Try a free lesson
30th Mar 2021, 12:25 AM
Mahdi Attaby Ibrahim
Mahdi Attaby Ibrahim - avatar
3 Answers
+ 3
Here are a couple improvements to get you started: 1. Pass your messages to the user without using alert. alert gets annoying from a laptop while resizing the display since Sololearn reruns your code on resize. Rerunning the code frequently means I need to click both your alerts to close them before doing anything else on the page. 2. Add target="_empty" to your "also try my flag game" link so it doesn't open everything in Sololearn's iframe. Regarding the code quality, put all the CSS in the CSS tab and JavaScript in the JS tab. Nearly all the JavaScript from the HTML tab can be refactored as CSS and you just use JavaScript to add or remove a class corresponding with each country.
30th Mar 2021, 5:11 AM
Josh Greig
Josh Greig - avatar
+ 1
Hachisenju wrote, "Check it now" Response: Looks much better. The annoying alerts are gone and your hyperlink goes to a completely separate page instead of loading in the iframe. I also see what was in a style element in the HTML tab is now in the CSS tab. What I suggested about converting JavaScript to CSS would move over 1000 lines and make the remaining JavaScript easier to understand. I'll show you some specific changes so Latvia's styles are moved completely to CSS. Hopefully this is clear enough that you see how the other countries can also be refactored in a similar way. If my suggestion sounds difficult, check this example: // The following JavaScript can be converted to CSS: couche.style.borderTop="solid firebrick 55px"; couche.style.borderBottom="solid firebrick 55px"; couche.style.borderLeft="solid red 0px"; couche.style.borderRight="solid red 0px"; couche.style.width="200px"; couche.style.height="40px"; couche.style.background="white"; point.style.display="none"; point2.style.display="none"; Here is the corresponding CSS: #flag.latvia #couche { border-top: solid firebrick 55px; border-bottom: solid firebrick 55px; border-left: solid red 0px; border-right: solid red 0px; width: 200px; height: 40px; } #flag.latvia #point, #flag.latvia #point2 { display: none; } Along with adding the above CSS to your CSS tab, you would add this to the top of your show function: function show(){ flag.className = ""; couche.style = ''; point.style = ''; point2.style = ''; Also, remove your inline style JavaScript for latvia like this: else if(reponse=="LATVIA"){ flag.classList.add('latvia'); }
30th Mar 2021, 3:14 PM
Josh Greig
Josh Greig - avatar
0
Check it now
30th Mar 2021, 10:15 AM
Mahdi Attaby Ibrahim
Mahdi Attaby Ibrahim - avatar