Skip to content

Conversation

@kaspnilsson
Copy link
Owner

This was a fuckton of work.

Theres at least one super obvious thing still left to do, which is to make navigating btw lessons in a path good. What "good" is is definitely open for discussion, I'm gonna start by adding a next / prev button to the lesson and we can go from there

@kaspnilsson kaspnilsson requested a review from malvidah January 8, 2022 02:11
@vercel
Copy link

vercel bot commented Jan 8, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/kaspnilsson/curyte/EUBrwFY65zDzFZBfeW8orvbdFo1y
✅ Preview: https://curyte-git-paths-kaspnilsson.vercel.app

Copy link
Collaborator

@malvidah malvidah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting a 500 error. Comes up on different pages and after different actions. Ex. sometimes I get to the account page and it comes up. Once I got to the path creation page and it came up. Sometimes I click on home page and it comes up.

console log just says things like:
lessons:1 Failed to load resource: the server responded with a status of 500 ()

Let me know how I can be more specific. I can tell you which buttons I click but it seems to differ slightly each time I get on the page.

@kaspnilsson
Copy link
Owner Author

Oof, looks like theres some unrelated problem with firebase on vercel: firebase/firebase-js-sdk#5823

Not sure why it's hitting this PR but not main...

Copy link
Collaborator

@malvidah malvidah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and works well:

  • creating a path on mobile and laptop (chrome, ios, mac os)
  • adding new lessons
  • adding existing lessons using search
  • viewing a path when not logged in using a shared link
  • viewing a path when logged in
  • adding new units
  • deleting units

To address before merge:

  • once i have created a path there is no button to add another path, there is only that one path that i can click into to view / edit.

Other notes that we may want to address later (not now because we want to just move on with the minimum version of this):

  • some way to navigate through lessons in a path, maybe just starting with next and previous buttons.
  • order is not clear when viewing paths and lessons are in card format. paths viewing as cards is nice but it is not the same as in edit mode where it shows as list view. i think from earlier convo let's keep it one way in both edit and view (my preference is for cards because it gives learners more information about what they are diving into and what topics it will cover, but we would have to make it clear (by putting cards in a row?) and you can drag drop in edit? or stick to list view in edit and view for now if that is too much work).
  • remove header text saying "units" and "lessons". not super important right now but probably unnecessary and should be easy to remove.
  • learners can't search to find paths. that's okay for now as long as learners can see the path that I send them with a link. It gives us some room to think about how we want users to find paths or if we want them to be organized by teachers and delivered to students that way.
  • eventually probably want the same hamburger menu grey icon button on paths for delete, share, edit, feature, etc. to keep styling coherent.

@kaspnilsson
Copy link
Owner Author

some way to navigate through lessons in a path, maybe just starting with next and previous buttons.
agreed, noted in OP.

order is not clear when viewing paths and lessons are in card format. paths viewing as cards is nice but it is not the same as in edit mode where it shows as list view. i think from earlier convo let's keep it one way in both edit and view (my preference is for cards because it gives learners more information about what they are diving into and what topics it will cover, but we would have to make it clear (by putting cards in a row?) and you can drag drop in edit? or stick to list view in edit and view for now if that is too much work).
I agree with this totally, but I think its actually a structural problem with the choice of cards -- i really do not like cards, it feels like the UI is tiled and it does not need to be. Let me propose some alternatives

remove header text saying "units" and "lessons". not super important right now but probably unnecessary and should be easy to remove.
I think this makes sense in the "view path" view, but not in the "edit path" view -- and i agree w your earlier point that the edit and view views (lol) should be as similar as possible. Not sure how to reconcile that with this

learners can't search to find paths. that's okay for now as long as learners can see the path that I send them with a link. It gives us some room to think about how we want users to find paths or if we want them to be organized by teachers and delivered to students that way.
Correct. This was a FUCKTON of work without searching, searching will DEFINITELY take longer.

eventually probably want the same hamburger menu grey icon button on paths for delete, share, edit, feature, etc. to keep styling coherent.
Again, agreed but that styling looks weird in the edit view. Not sure how to reconcile, might be worth taking a holistic approach and redesigning both views slightly or something.

Copy link
Collaborator

@malvidah malvidah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove header text saying "units" and "lessons". not super important right now but probably unnecessary and should be easy to remove.
I think this makes sense in the "view path" view, but not in the "edit path" view -- and i agree w your earlier point that the edit and view views (lol) should be as similar as possible. Not sure how to reconcile that with this
I think this can be addressed with placeholder text "Add a unit title...". The buttons already say "add lesson" so no need for a lesson header.

@malvidah malvidah merged commit dbe1f10 into main Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants