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

Add action to decline expense invite and display expense invite notes #10645

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hdiniz
Copy link
Contributor

@hdiniz hdiniz commented Sep 9, 2024

Related opencollective/opencollective#7535
Require opencollective/opencollective-api#10307

Description

Users invited to an expense should be presented with the invitation notes to give context, and they should be able to decline the invite with some comment.

@hdiniz hdiniz self-assigned this Sep 9, 2024
Copy link

vercel bot commented Sep 9, 2024

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

Name Status Preview Comments Updated (UTC)
opencollective-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 7:41am
opencollective-styleguide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 17, 2024 7:41am

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

@hdiniz The code looks good to me, I haven't tried the feature itself. Let me know when the API is ready & I'll test both!

components/expenses/graphql/queries.ts Show resolved Hide resolved
@hdiniz
Copy link
Contributor Author

hdiniz commented Sep 10, 2024

@hdiniz The code looks good to me, I haven't tried the feature itself. Let me know when the API is ready & I'll test both!

The API is ready for review. It covers the decline action and notifications for updates to the invited expense. The front end is currently functional as a sketch, I will iterate on the UX.

fyi: the child collective search will be on a different PR (wip)

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

Button positioning

Are we sure about the button positioning? It seems a bit off to me:
image

As a simple fix, we could simply align it with the title: image

But I would argue that this is part of a larger problem: people who arrive here have little to no context about what Open Collective is, so it could be nice to help them understand by adding a small message (if !LoggedInUser && Boolean(draftKey)). We can surely re-use the design here to do something like:

image

Max height on message

To avoid getting stuck with no ability to edit, submit or cancel.

Kooha-2024-09-11-11-38-39.mp4

Related issues found while testing (not related to this PR):

components/expenses/ExpenseForm.js Outdated Show resolved Hide resolved
components/expenses/DeclineExpenseInviteButton.tsx Outdated Show resolved Hide resolved
components/expenses/DeclineExpenseInviteButton.tsx Outdated Show resolved Hide resolved
@hdiniz
Copy link
Contributor Author

hdiniz commented Sep 17, 2024

Made changes to align with the new expense design:
image

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.

2 participants