Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fulfill #244

Merged
merged 24 commits into from
Aug 21, 2024
Merged

fulfill #244

merged 24 commits into from
Aug 21, 2024

Conversation

SheepTester
Copy link
Member

@SheepTester SheepTester commented Apr 30, 2024

Info

Closes #225

fulfills request for fulfill button

Changes

  • fulfill button for each order
  • checkboxes for items that can be deselected to mark as not fulfilled
  • got rid of fulfill view

Type of Change

  • Bug Fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • Logistics Change (A change to a README, description, or dev workflow setup like
    linting/formatting)
  • Continuous Integration Change (Related to deployment steps or continuous integration
    workflows)
  • Other: (Fill In)

Testing

I have tested that my changes fully resolve the linked issue ...

  • locally on Desktop.
  • on the live deployment preview on Desktop.
  • on the live deployment preview on Mobile.
  • I have added new Cypress tests that are passing.

Checklist

  • I have performed a self-review of my own code.
  • I have followed the style guidelines of this project.
  • I have documented any new functions in /src/lib/* and commented hard to understand areas
    anywhere else.
  • My changes produce no new warnings.

Screenshots

image

image

Copy link

vercel bot commented Apr 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
membership-portal-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 3:37am
testing-membership-portal-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 3:37am

Copy link
Contributor

@alexzhang1618 alexzhang1618 left a comment

Choose a reason for hiding this comment

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

Code looks good! Good call on removing the order fulfill view i was just parroting off of v1 even though we don't really need it

Some usability comments:

can we put some gap between the status and fulfill button and maybe make the button the same size as the fulfill tag
Screenshot 2024-05-10 at 3 37 34 PM

also i think it would be nice to sort by status (i.e. put orders that still need action at the top)

I think we discussed this but I think ux wise it makes sense for all checkboxes to start blank and the merch person will tick each box off as they get the item

Another thing to consider: is it better to put the fulfill button under the merch items for each user? I think it's nicer so your mouse only has to stay on one side but idk

src/components/admin/store/PickupOrder/index.tsx Outdated Show resolved Hide resolved
@SheepTester
Copy link
Member Author

I changed the orders view to use badges instead. not sure if it's better

image

Marcelo cancelled this rescheduled partially fulfilled order, which is why it has one fulfilled item:

image

Copy link
Contributor

@alexzhang1618 alexzhang1618 left a comment

Choose a reason for hiding this comment

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

I like the tags and the extra text, it looks nice!
Some UI comments:

I think I prefer ✅ and ❌ emojis to the text since it's more concise and readable

I'm also not sure how much I like having all these check boxes to the right of the item. In our store, we don't really have a lot of items where people are ordering a big quantity of them, but maybe we should make this just one check to fulfill all or maybe a number to fulfill a specific number of them? Up to you and whatever looks best
Screenshot 2024-05-18 at 12 26 44 PM

src/components/admin/store/PickupOrder/index.tsx Outdated Show resolved Hide resolved
src/components/store/OrderSummary/index.tsx Outdated Show resolved Hide resolved
src/components/store/OrderSummary/index.tsx Outdated Show resolved Hide resolved
@SheepTester
Copy link
Member Author

  • I switched the text labels in the admin orders page back to emoji

  • I merged all the checkboxes into one checkbox per variant. This means that if someone orders multiple ACM AI stickers, but there's somehow only one sticker left, you can only either mark them all as fulfilled or not fulfilled. Maybe that's fine for now.

  • I changed the "picked up" / "already picked up" (for rescheduled orders) badge to green:

    image

Copy link
Contributor

@alexzhang1618 alexzhang1618 left a comment

Choose a reason for hiding this comment

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

LGTM, some nits

Maybe a word break and a little margin between the checkbox n the button?
Screenshot 2024-08-19 at 11 01 14 PM

src/components/admin/store/PickupOrder/index.tsx Outdated Show resolved Hide resolved
@SheepTester
Copy link
Member Author

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin - Fulfill Pickup Event Items
2 participants