freeCodeCamp icon indicating copy to clipboard operation
freeCodeCamp copied to clipboard

feat: add progress bar to lower jaw

Open Sembauke opened this issue 2 years ago • 9 comments

  • feat add lower jaw back
  • fix: merge conflicts lower jaw and progress bar

Checklist:

Closes #50386

Sembauke avatar Apr 24 '23 12:04 Sembauke

:eyes: Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

ghost avatar Apr 24 '23 12:04 ghost

Thanks for working on this. Let us make sure we give credit to @Ismailtlem before merging.

ahmaxed avatar Apr 24 '23 12:04 ahmaxed

@Sembauke In the LowerJawStatus function, can we tack on the percentage complete message in sr-only text immediately after the congratulationText? So that screen reader users will hear the congrats message and then "N% complete."

bbsmooth avatar Apr 25 '23 20:04 bbsmooth

@Sembauke In the LowerJawStatus function, can we tack on the percentage complete message in sr-only text immediately after the congratulationText? So that screen reader users will hear the congrats message and then "N% complete."

I will look into it!

Sembauke avatar Apr 26 '23 09:04 Sembauke

So here are a few issues that I came across with the progress bar.

  • To apply the animation on every step, the applyAnimation and percent variables should be set to states and updated accordingly.
  • Since the progress bar is connected to redux, completedPercent updates after each challenge completion. As a result, window.setInterval runs without progress bar being on the screen. One solution could be to check if the component is on the view port before setting the interval. The might be a better a better solution, so I am open to suggestions.

ahmaxed avatar Apr 30 '23 17:04 ahmaxed

The progress bar should be working as expected. The component could be simplified further, but seems out of scope for this pr.

ahmaxed avatar May 05 '23 10:05 ahmaxed

@Sembauke feel free to add a couple of e2e tests here and open the pr for review.

ahmaxed avatar May 09 '23 10:05 ahmaxed

can we tack on the percentage complete message in sr-only text immediately after the congratulationText?

If we want to push this through now without this, that's fine. I realize this is not a simple update, so I'll fix it after the fact. I just wanted to bring it up again because it's in my nature to be a pest.

bbsmooth avatar May 16 '23 23:05 bbsmooth

Sorry this wasn't ready for review. There still seem to be some issues.

Sembauke avatar May 17 '23 07:05 Sembauke

Thank you @Sembauke for your hard work trying to get this one in. I tested this pr, but didn't come across an issue. I also had a chance to look into the inconsistency that you mentioned in chat about the percentage turning to 1 after the tests run on the 3rd challenge. Could you please double check if the issue still exist and if there are other issues that you want me to look into?

ahmaxed avatar Jun 05 '23 09:06 ahmaxed

Thank you @Ismailtlem and @Sembauke for your hard work. We have a lower jaw progress bar now :)

ahmaxed avatar Jun 06 '23 12:06 ahmaxed