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

Flag options #462

Merged
merged 4 commits into from
Oct 12, 2020
Merged

Flag options #462

merged 4 commits into from
Oct 12, 2020

Conversation

mahboubii
Copy link
Contributor

@mahboubii mahboubii commented Oct 8, 2020

closes #314

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2020

Size Change: +325 B (0%)

Total Size: 197 kB

Filename Size Change
dist/browser.es.js 41.7 kB +89 B (0%)
dist/browser.js 42.3 kB +75 B (0%)
dist/index.es.js 41.8 kB +88 B (0%)
dist/index.js 42.3 kB +73 B (0%)
ℹ️ View Unchanged
Filename Size Change
dist/browser.full-bundle.min.js 28.4 kB 0 B

compressed-size-action

Comment on lines -689 to -697
export type FlagMessageOptions<UserType = UnknownType> = {
client_id?: string;
connection_id?: string;
created_by?: string;
target_message_id?: string;
target_user_id?: string;
user?: UserResponse<UserType>;
user_id?: string;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like this specific type overcomplicates the functions for users, the only thing that is really needed here is user_id?: string

@mahboubii mahboubii requested a review from DanC5 October 8, 2020 08:39
Copy link
Contributor

@DanC5 DanC5 left a comment

Choose a reason for hiding this comment

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

LGTM, and at some point we should probably add documentation about the different params used server-side. We currently have a tiny Server Side API docs section but it misses most of these use cases.

@mahboubii mahboubii merged commit 537ac55 into master Oct 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the flag-options branch October 12, 2020 08:32
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.

Missing server side support for Flag / Unflag
3 participants