seamapi / react

Seam Components are a set of white-labeled UI elements that can be added to your application in seconds. Use them in any app as native web components or as React components and hooks.
https://react.seam.co
MIT License
6 stars 2 forks source link

Add `disableConnectedAccountInformation` prop to common props #610

Closed dawnho closed 2 weeks ago

dawnho commented 2 months ago

For End users who don't own the device, we don't want to show them the account email address, and device ID.

To-dos:

This section is what we want to hide: Screen Shot 2024-03-15 at 10 35 45 AM

dawnho commented 2 months ago

Some ideas, we can rename this section to "Device information", the property could be renamed to disableDeviceInformation.

Thoughts? @razor-x @xplato

razor-x commented 2 months ago

Is the manufacturer useful? We already have disableResourceIds which covers one of the fields. What if we add disableConnectedAccountPersonalIdentifiableInformation? or disableConnectedAccountPii?

dawnho commented 2 months ago

For the resident, probably not, I don't think they need to know the manufacturer. They're just interested in the thermostat controls and seeing the current temperature / settings

dawnho commented 2 months ago

@razor-x To hide that whole section, would they just need to set both disableConnectedAccountInformation and disableResourceIds to `true?

razor-x commented 2 months ago

Ok, I've updated the title, we can add disableConnectedAccountPii, but I don't think it makes sense for this to control the manufacturer.

To hide both the email / id, they would set disableResourceIds and disableConnectedAccountPii.

I still think hiding this entire section is the same case as https://github.com/seamapi/react/pull/607 and is out of scope of the prop API. The API focuses on controlling functionality and behaviors, not tweaking UI at the component level from the global scope.

In my opinion, we should not add a new prop. Even disableDeviceInfo is ambiguous (we don't even title that section wit info) and instead:

  1. Re-title this section to Device Information.
  2. Add a CSS class seam-device-details-row-information to this section.
  3. The user can set a CSS hidden property for seam-device-details-row-information.
dawnho commented 2 months ago

@razor-x I kind of prefer disableConnectedAccountInformation to disableConnectedAccountPii. Pii is an acronym, and not all the information being hidden is personally identifying.

razor-x commented 3 weeks ago

@dawnho Looking at this with fresh eyes, this row only exists on ThermostatDeviceDetails. We should either remove it entirely from the component, or add it consistently to all device detail components and add the prop to disable it.

              <DetailRow
                label={t.linkedAccount}
                sublabel={
                  connectedAccount?.user_identifier?.email ??
                  device.connected_account_id
                }
              />
dawnho commented 3 weeks ago

I think this can be valuable for all device types. This section shows the device id and connected account info

razor-x commented 2 weeks ago

@dawnho This is now available in version 2.16.0. There are two options:

  1. To hide the connected account email and resource ids everywhere, use the common props disableConnectedAccountInformation and disableResourceIds
  2. To hide the entire "device info section" only, use a CSS style like .seam-device-details-device-info { display:none; }