Local data experiment #2
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… theme does not get selected or randomised when it has been selected or randomised once before.
…from the selectable or randomisable themes until the themes array is empty.
…n list, grayed out
…s now get stored in local storage in an array called usedEmployees
… hook that stores theme and employees instead of doing it in app.js
…domised cannot be chosen/randomised again, until their respective arrays run out, then they begin anew. Everything works as expected except that people who are re-randomised through dice press cannot get randomly become an office host again.
…erency of styling.
…eleted unnecessary comments.
…h the user manually choosing a theme from the dropdown menu, the de-selected theme re-enters the original themes array and disappears from the localStorage, so it can be both randomised and picked again.
…tored themes array rather than the second to last theme when a user manually selects a new theme from the dropdown.
…ser through picking a new theme from the dropdown menu is now put back into the choosable and randomisable themes array.
… press is now removed from the local storage, yet to be added back into the employees array.
…that splices the clonedThemes that exist in storedThemes isn't waterproof, sometimes themes that do exist in storedThemes suddenly pop up in clonedThemes as well... one person can become both of the office hosts because the chosen employees don't get spliced out of the clonedEmployees array until both office hosts have been set. Even though an employee has become an office host and is set in storedEmployees they can still become office host once again, and I think it's because clonedEmployees takes in the whole employees array each render?
…fice hosts into a switch case. All the bugs are still there + I uncovered a new one: if there are office hosts stored in the local storage prior to when a user re-randomises an office host through a dice press the entire local storage of employees is replaced with the current office hosts.
…emoved from the themes array from which the user can choose and randomise new themes from.
…ployees array so users cannot choose or randomise them again.
…s array is safely restored, making the tombola re-start
…themes have been used up.
…ution to removing a deselected person from the local storage and storedEmployees array.
…Array to make them available for a re-randomisation
… new theme from the dropdown are now removed from local storage and storedThemes
…me manually from the dropdown are now re-added into the themesArray. The issue with identical keys within the dropdown is also solved. Similarily to the stored employees the themes are now transformed into only the property of theme rather than the whole object.
claraiskssn
requested review from
moke20 and
simonahlstrom
and removed request for
moke20
… of employees and instead tried to be explicit with that these are not employees, these are how the person components should be rendered when no employee has been chosen, when the employee is loading and when the employee is set to an actual employee. Don't know if the code became better or worse really. It felt like it became longer
…ing the user to randomise both office hosts simultaneously)
… to get stored multiple times if the app re-renders + cleaned up a few other things
…otential code reviews.
claraiskssn
requested review from
LiamMelin,
antonsjoo,
b3cka,
mackan92,
moke20,
simonahlstrom and
trollcus
…s when the user clicks on a person. (also did some minor refactoring)
…e it was going bonkers and re-rendered too many times.
…tton but left the two action buttons that replace the randomise button without padding.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello
Detta är min tema- och kontorsvärdsgenerator som använder sig utav localStorage för att hålla koll på vilka teman och kontorsvärdar som varit så att de inte blir igen förrän alla varit. Men min kod är lite ful tycker jag! Jag skulle uppskatta något så oerhört om ni ville kika igenom koden och peka ut lite grejer vad jag gör för fel och vad jag kan göra annorlunda/tänka på för att bli bättre!
Jag har markerat i koden (den senaste committen) vart jag speciellt behöver hjälp, markeringen ser ut såhär:
/*
! CODE REVIEW !
blablabla
*/
koden jag behöver hjälp med
Förstår om ni har ont om tid för att hålla på o hjälpa mig med sånt här, men skulle tid finnas någon gång så skulle jag bli jätteglad som sagt ⭐