-
Notifications
You must be signed in to change notification settings - Fork 178
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
Remove commands we don't want to support #13
Conversation
6998928
to
6eb9aa5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeeeet lgtm
6eb9aa5
to
7cf96c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 comment
yeet
docs/common-patterns/layout-mode.md
Outdated
export default function LayoutMode() { | ||
const handleLayoutModeUpdate = React.useCallback((update: {layout_mode: number}) => { | ||
const handleLayoutModeUpdate = React.useCallback((update: EventPayloadData<"ACTIVITY_LAYOUT_MODE_UPDATE">) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer the const for the action name Events.ACTIVITY_LAYOUT_MODE_UPDATE
, here and below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to import as a type, i.e.
import type {Events, EventPayloadData} from '@discord/embedded-app-sdk';
type Foo = EventPayloadData<Events.ACTIVITY_INSTANCE_PARTICIPANTS_UPDATE>['participants']
As-is we get autocomplete and it will throw an error if it's not a valid string. Do we feel strongly about enforcing that style? I'm (strong opinion, loosely held) pro using strings because of the smaller size and less footguns around importing an enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per convo, this doc is migrated to the main docs site, but we can explore updating our patterns in the docs outside the scope of this PR
7cf96c6
to
cdb26cc
Compare
No description provided.