freeCodeCamp / CurriculumExpansion

Creative Commons Attribution Share Alike 4.0 International
313 stars 105 forks source link

fix: suggested changes for todo app #348

Closed scissorsneedfoodtoo closed 1 year ago

scissorsneedfoodtoo commented 1 year ago

Checklist:

Closes #XXXXX

These updates are based on a recent discussion with Oliver, and should hopefully make it more clear which elements learners are working with while writing JS. I also added a bit of responsiveness as well, and tried to simplify the structure and styling as much as possible.

Also, it adds some code to close the dialog modal with the .close() method based on Jessica's suggestion. This should help us get around some platform limitations.

More context here: https://chat.freecodecamp.org/group/front-3-2023?msg=vkafmcPdmbadnDM39

Ksound22 commented 1 year ago

Hey Kris 👋🏽

Thanks a lot for looking into this.

I did some testing on my end. The modal now closes, but the initial error in the console persists:

https://github.com/freeCodeCamp/CurriculumExpansion/assets/65571316/dfdfe2bd-e70e-4420-8116-c6658286dda5

If that won't cause any further issues then we are good to go.

scissorsneedfoodtoo commented 1 year ago

Hey @jdwilkin4 and @Ksound22, thanks for your patience. This should be ready for testing now.

@jdwilkin4, there are now some checks for whether the inputs have values in them when adding a new task, or whether the values have been updated when editing a task, so we may be able to leave the modal text as is. But if you'd like to update the text from "Discard current changes?" to something else, please let me know.

@Ksound22, we should be able to ignore that error for now. That's coming from the check in client/src/templates/Challenges/utils/frame.ts, and we should be able to add optional chaining there to prevent the error for this project without causing issues for existing projects.

Ksound22 commented 1 year ago

Those are some good improvements, Kris.

Things are good to go on my end!