fix: Accordion state #1544

SLimbu30 · 2023-11-28T15:30:28Z

Description

Changes in the accordion state when being clicked.
Tracks the accordion state according to the level and episode.

How Has This Been Tested?

Locally in my computer

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have linked this PR to a ZenHub Issue.

This change is

codecov · 2023-11-28T15:35:56Z

Codecov Report

Merging #1544 (804ed12) into master (e412665) will not change coverage.
The diff coverage is n/a.

❗ Current head 804ed12 differs from pull request most recent head 8223e44. Consider uploading reports for the commit 8223e44 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1544   +/-   ##
=======================================
  Coverage   91.79%   91.79%           
=======================================
  Files         110      110           
  Lines        5860     5860           
=======================================
  Hits         5379     5379           
  Misses        481      481

faucomte97

Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @karatmate)

game/static/game/js/game.js line 24 at r2 (raw file):

document.addEventListener("DOMContentLoaded", function() {
  var dataElement = document.getElementById('data');

var is an old way of declaring variables in Js - instead, you can use let and const. Please use:

let when the variable needs to be changed later on in the code.
const when declaring a constant which only needs to be read / queried and not changed.

Please perform this change throughout the code you've added.

game/static/game/js/game.js line 26 at r2 (raw file):

  var dataElement = document.getElementById('data');
  var currentEpisode = dataElement ? dataElement.getAttribute('data-episode') : null;

You've got some extra whitespaces here - please remove them 🙂

game/static/game/js/game.js line 36 at r2 (raw file):

  var currentLevelId = LEVEL_ID; 
  var currentEpisode = this.determineEpisodeFromLevel(currentLevelId);
  localStorage.setItem('currentEpisode', currentEpisode);

Here you are getting the level ID, then determining the episode based on the level ID.
LEVEL_ID comes from game.html.
You'll notice that game.html also has a var called EPISODE - maybe you can get the ID directly from that instead of getting the LEVEL ID, and that way you might not even need the determineEpisodeFromLevel function anymore.

Code quote:

  var currentLevelId = LEVEL_ID;
  var currentEpisode = this.determineEpisodeFromLevel(currentLevelId);
  localStorage.setItem('currentEpisode', currentEpisode);

game/templates/game/game.html line 92 at r2 (raw file):

  });
  
</script>

Please double-check whether you need this, it seems you're already doing that in game.js anyway, right?

Code quote:

<script>
  document.addEventListener("DOMContentLoaded", function(){
      var dataElement = document.getElementById('data');
      var currentEpisode = dataElement ? dataElement.getAttribute('data-episode') : null;

      if (currentEpisode) {
          localStorage.setItem('currentEpisode', currentEpisode);
      }
  });

</script>

game/templates/game/level_selection.html line 44 at r2 (raw file):

</script>

<script>

Since level_selection.js is imported in the line directly after this script, could you try moving this JS code to level_selection.js and see if it still works? In general, when following this kind of software architecture, it's better to keep JS code in JS files instead of HTML files.

…apid-router into Project-rapid-router

SLimbu30

Reviewable status: 0 of 4 files reviewed, 5 unresolved discussions (waiting on @faucomte97)

game/static/game/js/game.js line 24 at r2 (raw file):