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

[WIP][Task 8700131] Implementing pages.responseButton sub-capabilities #2157

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MengyiGong
Copy link
Contributor

For more information about how to contribute to this repo, visit this page.

Description

Summarize the changes, including the goals and reasons for this change. Be sure to call out any technical or behavior changes that reviewers should be aware of.

If this Pull Request should close/resolve any issues when merged, use the special syntax for that here.

Main changes in the PR:

  1. <Change 1>
  2. <Change 2>

Validation

Validation performed:

  1. <Step 1>
  2. <Step 2>

Unit Tests added:

Unit tests are required for all changes. If no unit tests were added as part of this change, please explain why they aren't necessary.

<Yes/No>

End-to-end tests added:

<Yes/No>

Additional Requirements

Change file added:

Ensure the change file meets the formatting requirements.

<Yes/No>

Related PRs:

Remove this section if n/a

Next/remaining steps:

List the next or remaining steps in implementing the overall feature in subsequent PRs (or is the feature 100% complete after this?).

Remove this section if n/a

  • Item 1
  • Item 2

Screenshots:

Remove this section if n/a

Before After
< image1 > < image2 >

@MengyiGong MengyiGong self-assigned this Feb 2, 2024
Copy link
Contributor

github-actions bot commented Feb 2, 2024

This pull request contains changes to the runtime.ts file. If you, as the author of this PR, have made changes to the Runtime interface please review RUNTIME.md to determine if a new runtime version is required. Please reply to this comment stating what changes, if any, were made to the runtime object and whether a new runtime version was required.

packages/teams-js/src/public/pages.ts Outdated Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Outdated Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Outdated Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Outdated Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Outdated Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Show resolved Hide resolved
packages/teams-js/src/public/pages.ts Show resolved Hide resolved
*
* @beta
*/
export function responseButtonEventHandler(appEventHandler: handlerFunctionType): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there no data the host has to pass down to the app developer when this handler is triggered? For example, if the response button is a button with a dropdown that presents multiple options (forward, replyAll, etc.) do you need to make an enum or something similar available here?

Copy link

Choose a reason for hiding this comment

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

I remember from earlier discussion(like a year ago) we did not to want to maintain a list of enums (like: reply, replyall, forward), due to the reason that if hubs decide to add more dropdown options, we will need to add to this list and needs SDK changes. So, it is hard to scale and maintain in the long term.

If we want to allow developers know which button got clicked, and able to handle different logic based on which button got clicked. That sounds good to me. My only ask is that developer don't need to register 3 handlers for 3 items in the same dropdown item if they share the same handler logic.

packages/teams-js/src/public/pages.ts Show resolved Hide resolved
@MengyiGong MengyiGong force-pushed the maggieg/add_responseButton_api branch from 700ee2f to 1a235c2 Compare February 9, 2024 04:47
@MengyiGong MengyiGong force-pushed the maggieg/add_responseButton_api branch 5 times, most recently from 4c8d65a to 86a313d Compare February 15, 2024 02:01
Co-authored-by: Trevor Harris <trharris@microsoft.com>
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.

None yet

3 participants