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

docs(react-keytips): add detailed Overflow example #273

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

mainframev
Copy link
Contributor

@mainframev mainframev commented Dec 18, 2024

Added more detailed Overflow example, so it looks more closer to later usage by partners. Previous Overflow example renamed to Shortcut as it mainly focuses on showing the shortcut behaviour.

Screen.Recording.2024-12-19.at.15.58.58.mov

@mainframev mainframev requested a review from a team as a code owner December 18, 2024 20:18
@mainframev mainframev self-assigned this Dec 18, 2024
@mainframev mainframev force-pushed the docs/add-complete-example branch 3 times, most recently from 863ef0b to 442bba4 Compare December 18, 2024 21:17
disabled: false,
name: 'Feedback',
keytipProps: keytipsMap.feedback,
// icon: <NotepadPersonIcon />,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: probably, this comment is not needed

Suggested change
// icon: <NotepadPersonIcon />,

Comment on lines 6 to 12
MenuButtonProps,
MenuList,
MenuItem,
MenuPopover,
MenuTrigger,
SplitButton,
SplitButtonProps,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MenuButtonProps,
MenuList,
MenuItem,
MenuPopover,
MenuTrigger,
SplitButton,
SplitButtonProps,
type MenuButtonProps,
MenuList,
MenuItem,
MenuPopover,
MenuTrigger,
SplitButton,
type SplitButtonProps,

icon={<MailRegular />}
menuButton={{
...triggerProps,
// @ts-expect-error TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need TODO here?

OverflowStory.parameters = {
docs: {
description: {
story: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this part if it's empty?

Comment on lines 3 to 10
export const onExecute: ExecuteKeytipEventHandler = (_, { targetElement }) => {
if (targetElement) {
console.info(targetElement);
targetElement.focus();
targetElement.click();
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to move this method to some shared file, because it's duplicated in Home and Help folders.

@mainframev mainframev force-pushed the docs/add-complete-example branch from f37a6b5 to 449020e Compare December 27, 2024 12:19
@mainframev mainframev merged commit 750409f into microsoft:main Dec 27, 2024
3 checks passed
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.

2 participants