whereby / sdk

MIT License
11 stars 4 forks source link

browser-sdk: open_settings command #337

Closed fuo213 closed 2 months ago

fuo213 commented 2 months ago

Description

Summary:

This is a command for the embed element that allows developers to set up their own button to open the settings pane.

Related Issue:

None

Testing

  1. Create a localstack room to use in story book
  2. Add a supported command for the button under sdk/packages/browser-sdk/src/stories/prebuilt-ui.stories.tsx
  3. Run yarn dev and navigate to Prebuilt-UI Stories -> Embed Element Example
  4. Click button, settings page should open to the defined pane

Screenshots/GIFs (if applicable)

N/A

Checklist

Additional Information

Supported values for this command are any of the following [theme, integrations, streaming, effects, notifications, advanced, media]

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 2609995f7f7b9fef1a1691c67b107bf34ea289e8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------------ | ----- | | @whereby.com/browser-sdk | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

fuo213 commented 2 months ago

One question I have around this command is currently nothing will happen if a value is not passed in or an unsupported value is passed in. I could see a few options here

  1. Add some sort of validation to ensure only supported values are passed in and/or create error handling to notify users of issues with the supplied value
  2. Add a default value (probably "media")

Not totally sure on the best way to do this, and think this would be a good learning opportunity for myself and Nick if a kind developer would be willing to weigh in on this.

@thyal @kevinwhereby maybe?

kevinwhereby commented 2 months ago

hmmmmm I'm not sure what the default settings pane would be ... I'd probably lean towards a console.warn message saying you need to pass one.

Or a more complicated solution would be toggle whatever the last opened pane was... but that's a whole other story. I defer to @thyal :D

thyal commented 2 months ago

console.warn sounds good. But we can also type the input, instead of accepting just string. Let me know if you want some help to look at that