oxalorg / 4ever-clojure

Pure cljs version of 4clojure, meant to run forever!
https://4clojure.oxal.org
230 stars 28 forks source link

feat: better problems modal #73

Closed harismh closed 1 year ago

harismh commented 1 year ago

Adds some opinionated tweaks to the problems modal.

I removed the close button at the bottom and replaced it with an "X" at the top right as I kept getting the primary action confused. For the primary action, it's now conditional based on the test results. If all tests pass, it shows a "Next problem #" button, otherwise it shows a return button.

I am still learning Clojure, so any suggestions on more idiomatic code is much appreciated!

Screenshot 2022-12-03 at 13 38 38 Screenshot 2022-12-03 at 13 38 44
oxalorg commented 1 year ago

Thanks for the PR @harismh 🙌

Unfortunately we've moved away from the modal paradigm for test runs. We do use it only when all tests pass. Do you want to rebase it on main and still keep the close button in there? (The primary CTA on the modal can be to go to the next problem).

I'll be happy to do this if and merge it if you do not have the time available! Thanks again :)

harismh commented 1 year ago

Hey oxalorg, thanks for the review. The new site looks nice, and I enjoy the solution history. I'll try the rebase approach, but if I find the changes too extensive, I'll open a slimmed-down PR with just the close and CTA changes.

harismh commented 1 year ago

@oxalorg Finished rebasing and migrating changes. Let me know if further changes are needed.

oxalorg commented 1 year ago

LGTM! Thanks a lot @harismh 🥳