feat(Sheet): new component #840

atabel · 2023-08-03T13:43:16Z

Figma: https://www.figma.com/file/nxToAYk7FGC86TS0mY61TU/%F0%9F%94%B8-Sheet-specs?type=design&node-id=3346-16622&mode=design&t=AfsHntMnROCmNvcD-0

Webview bridge spec: https://confluence.tid.es/display/CTO/%5BAPPS%5D+Shared+Spec%3A+Bottom+Sheet+opened+from+Webview-Bridge

github-actions · 2023-08-03T13:45:55Z

Size stats

	master	this branch	diff
Total JS	9.42 MB	9.45 MB	+36.4 kB
JS without icons	915 kB	951 kB	+36 kB
Lib overhead	127 kB	128 kB	+417 B
Lib overhead (gzip)	32.9 kB	33 kB	+100 B

github-actions · 2023-08-03T13:47:38Z

Deploy preview for mistica-web ready!

✅ Preview
https://mistica-hr3e49gef-tuentisre.vercel.app

Built with commit 355ae57.
This pull request is being automatically deployed with vercel-action

github-actions · 2023-08-03T13:49:28Z

Accessibility report
✔️ No issues found

ℹ️ You can run this locally by executing yarn audit-accessibility.

atabel · 2023-08-11T10:58:25Z

doc/bottomSheet.md

+    <>
+      <ButtonPrimary onPress={() => setShowSheet(true)}>show bottom sheet</ButtonPrimary>
+      {showSheet && (
+        <BottomSheet onClose={() => setShowSheet(false)}>


note that onClose is called when the closing animation finishes, so the idea is not unmounting the component until the transition finishes, that's why I set showSheet to false in onClose handler instead of ButtonPrimary onPress handler

atabel · 2023-08-11T11:00:52Z

doc/bottomSheet.md

+Mística predefines some common bottom sheet patterns for you to use: `RadioListBottomSheet`,
+`ActionsListBottomSheet`, `InfoBottomSheet` and `ActionsBottomSheet`. You can see examples in the storybook.
+
+## `showBottomSheet` imperative api


this equivalent to the confirm(), alert() methods we have for dialogs

atabel · 2023-08-11T11:07:13Z

doc/bottomSheet.md

+export const App = () => {
+  return (
+    <>
+      <BottomSheetRoot />


I've decided to not include this automatically inside ThemeContextProvider to allow webs using mistica to optin to use this. The advantage is that we don't bloat webs with all sheet components JS code if they are not going to use it. Also, it isn't needed to use BottomSheetRoot if you don't wan't to use the imperative api

atabel · 2023-08-11T11:09:55Z

doc/bottomSheet.md

+import {BottomSheetRoot} from '@telefonica/mistica';
+import * as webviewBridge from '@tef-novum/webview-bridge';
+
+const nativeImplementation = createNativeSheetImplementationFromWebviewBridge(webviewBridge);


with this approach, mistica doesn't need to depend on webview bridge lib, because the native implementation is injected, not imported. I think we could follow a similar approach for alert() confirm() or other components using the bridge. That would allow us to remove the "@tef-novum/webview-bridge" dependency from mistica package.json

Ideally we should be able to inject the bridge function "as is" to the Root component.

Something like:

import {openBottomSheet} from '@tef-novum/webview-bridge`; <BottomSheetRoot nativeImplementation={openBottomSheet} />

and the nativeImplementation prop would be typed as typeof openBottomSheet;

This way everything is tied, types are verified at compile time and we won't need a createNativeSheetImplementationFromWebviewBridge.

Perhaps the bridge dependency could be listed as optionalDependency. I think it would be a problem to fully decouple both implementations.

hmmm... I see your point, but that would mean I'd need to re-implement the specific sheet methods in mistica (bottomSheetSingleSelector, bottomSheetActionSelector, bottomSheetInfo, bottomSheetActions)

ok, I've moved these specific sheet methods from the bridge lib to mistica and removed them from webview-bridge (Telefonica/webview-bridge#115)

doc/bottomSheet.md

src/__stories__/bottom-sheet-story.tsx

src/bottom-sheet.tsx

playroom/frame-component.tsx

src/__stories__/bottom-sheet-story.tsx

src/bottom-sheet.tsx

doc/bottomSheet.md

pladaria · 2023-08-14T11:48:43Z

src/bottom-sheet-root.tsx

+type SheetPropsByType = {
+    RADIO_LIST: SheetProps<{
+        selectedId?: string;
+        items: Array<{
+            id: string;
+            title?: string;
+            description?: string;
+            icon?: {
+                size?: 'small' | 'large';
+                url: string;
+                urlDark?: string;
+            };
+        }>;
+    }>;
+    ACTIONS_LIST: SheetProps<{
+        items: Array<{
+            id: string;
+            title: string;
+            style?: 'normal' | 'destructive';
+            icon?: {
+                url: string;
+                urlDark?: string;
+            };
+        }>;
+    }>;
+    INFO: SheetProps<{
+        items: Array<{
+            id: string;
+            title: string;
+            description?: string;
+            icon: InfoIcon;
+        }>;
+    }>;
+    ACTIONS: SheetProps<{
+        button: {
+            text: string;
+        };
+        secondaryButton?: {
+            text: string;
+        };
+        link?: {
+            text: string;
+            withChevron?: boolean;
+        };
+    }>;


all this could be imported from the bridge, which should be the source of truth for this kind of components

pladaria · 2023-08-14T12:24:11Z

src/stack.tsx

        >
            {React.Children.map(children, (child) => (
-                <div>{child}</div>
+                <div role={role === 'list' ? 'listitem' : undefined}>{child}</div>


pladaria · 2023-08-14T12:28:58Z

src/radio-button.css.ts

-        transform: 'scale(1)',
+        transform: 'none',


what motivated this change?

scale(1) creates a new stacking context, that was affecting zindex inside sheet

src/bottom-sheet.tsx

pladaria · 2023-08-14T13:57:27Z

src/bottom-sheet.tsx

+
+            isDraggingRef.current = false;
+            setDragDistance(0);
+            if (dragDistance > 50 && (currentYRef.current > window.innerHeight * 0.75 || dragSpeed > 0.5)) {


Maybe 0.50 would be a better threshold (to close it you must drag a distance >= half the height)

I've been playing and I think the current values feel more natural, give it a try

src/bottom-sheet.tsx

atabel · 2023-08-21T15:04:54Z

Should we take into account a scenario in which the content overlaps with the close button? Playroom example

I don't think so. It's already managed by SheetBody. And if someone needs some special sheet behavior, it's their responsibility to use proper paddings to no overlap with the close button.

marcoskolodny

Using chrome iOS, it looks like when I scroll the sheet's content, it assumes I'm scrolling the sheet itself, but just for a few milliseconds. After that, it starts scrolling the content.

This happens only sometimes, seems unstable. I used this snippet in Playroom.

I couldn't reproduce this issue in desktop/android devices.

IMG_5784.MP4

atabel · 2023-08-22T08:08:54Z

Using chrome iOS, it looks like when I scroll the sheet's content, it assumes I'm scrolling the sheet itself, but just for a few milliseconds. After that, it starts scrolling the content.

This happens only sometimes, seems unstable. I used this snippet in Playroom.

I couldn't reproduce this issue in desktop/android devices.

IMG_5784.MP4

hmmm... it's dificult to reproduce, maybe you tapped the grab zone with your finger?

yceballost · 2023-08-22T09:10:43Z

Should we take into account a scenario in which the content overlaps with the close button? Playroom example

I don't think so. It's already managed by SheetBody. And if someone needs some special sheet behavior, it's their responsibility to use proper paddings to no overlap with the close button.

not really, If I take the Marcos's example and use sheetbody inside without title prop, the content do not respect the top padding
https://tinyurl.com/2c3nx866

Maybe we can solved this case using the same title padding for children when no props is added in SheetBody (?)

Component in Figma

atabel · 2023-08-22T14:14:13Z

Should we take into account a scenario in which the content overlaps with the close button? Playroom example

I don't think so. It's already managed by SheetBody. And if someone needs some special sheet behavior, it's their responsibility to use proper paddings to no overlap with the close button.

not really, If I take the Marcos's example and use sheetbody inside without title prop, the content do not respect the top padding https://tinyurl.com/2c3nx866

Maybe we can solved this case using the same title padding for children when no props is added in SheetBody (?)

Component in Figma

What happens with scroll in those cases?
something like this?

looks weird to me

yceballost · 2023-08-23T07:45:31Z

Should we take into account a scenario in which the content overlaps with the close button? Playroom example

I don't think so. It's already managed by SheetBody. And if someone needs some special sheet behavior, it's their responsibility to use proper paddings to no overlap with the close button.

not really, If I take the Marcos's example and use sheetbody inside without title prop, the content do not respect the top padding tinyurl.com/2c3nx866
Maybe we can solved this case using the same title padding for children when no props is added in SheetBody (?)
Component in Figma

What happens with scroll in those cases? something like this? looks weird to me

I would solve it exactly the same way as in the dialogs
https://tinyurl.com/2betx7tw

(Is the Divider between title and extra defined in specs? I can't see it in specs)

yceballost

🤩

# [14.21.0](v14.20.1...v14.21.0) (2023-08-24) ### Bug Fixes * **Cards:** remove min width from all the cards ([#858](#858)) ([029a6ea](029a6ea)) * **Tabs:** remove extra margin in safari ([#857](#857)) ([be8c2f2](be8c2f2)) ### Features * **FeedbackScreens:** updates in icons, paddings and styling ([#852](#852)) ([4b231e6](4b231e6)) * **Sheet:** new component ([#840](#840)) ([f03ca7c](f03ca7c)) * **utilities:** Remove lodash dependency and replace it with custom functions ([#856](#856)) ([dcaf691](dcaf691))

tuentisre · 2023-08-24T13:22:27Z

🎉 This PR is included in version 14.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

WEB-1492 feat(BottomSheet): new component

e5d5039

atabel changed the title ~~WEB-1492 feat(BottomSheet): new component~~ feat(BottomSheet): new component Aug 3, 2023

Merge branch 'master' into atoledano-sheet

858f52b

atabel added 6 commits August 4, 2023 08:33

forwardRef and refactor ActionsBottomSheet

7b36788

desktop support

d9995c3

fix title divider logic

99f4ca2

improve stories

81ff7b8

BottomSheetRoot

8b7f3d2

tests

046301b

atabel commented Aug 11, 2023

View reviewed changes

missing tests

03d6546

atabel requested review from yceballost, pladaria and marcoskolodny August 11, 2023 11:16

atabel marked this pull request as ready for review August 11, 2023 11:16

atabel added 2 commits August 11, 2023 16:03

Merge branch 'master' into atoledano-sheet

ca2b10e

Merge branch 'master' into atoledano-sheet

45496e9

yceballost requested changes Aug 14, 2023

View reviewed changes

pladaria reviewed Aug 14, 2023

View reviewed changes

doc/bottomSheet.md Outdated Show resolved Hide resolved

pladaria reviewed Aug 14, 2023

View reviewed changes

doc/bottomSheet.md Outdated Show resolved Hide resolved

pladaria reviewed Aug 14, 2023

View reviewed changes

pladaria approved these changes Aug 14, 2023

View reviewed changes

Merge branch 'master' into atoledano-sheet

ee13795

add subtitle and description to info snippet

52fc6a3

marcoskolodny reviewed Aug 21, 2023

View reviewed changes

fix bridge type

fb9a4a8

atabel added 3 commits August 22, 2023 11:34

fix text

515bd46

fix scrollbars in desktop]

00e2495

move specific sheet native methods from bridge to mistica

509ac2c

keep top padding in SheetBody when no title

1aad7a0

atabel mentioned this pull request Aug 22, 2023

BREAKING CHANGE: remove sheet specific methods Telefonica/webview-bridge#115

Open

link to doc in sheet story

60cf078

atabel and others added 3 commits August 23, 2023 11:48

snippets

1c842d9

remove divider in desktop

87f8616

Merge branch 'master' into atoledano-sheet

be5e558

atabel requested review from marcoskolodny, yceballost and aweell August 23, 2023 14:30

marcoskolodny approved these changes Aug 23, 2023

View reviewed changes

update snapshots

355ae57

yceballost approved these changes Aug 24, 2023

View reviewed changes

atabel added this pull request to the merge queue Aug 24, 2023

Merged via the queue into master with commit f03ca7c Aug 24, 2023
10 checks passed

atabel deleted the atoledano-sheet branch August 24, 2023 08:54

tuentisre added the released label Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Sheet): new component #840

feat(Sheet): new component #840

atabel commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

atabel Aug 11, 2023

atabel Aug 11, 2023

atabel Aug 11, 2023

atabel Aug 11, 2023

pladaria Aug 14, 2023 •

edited

Loading

atabel Aug 21, 2023

atabel Aug 22, 2023

pladaria Aug 14, 2023 •

edited

Loading

pladaria Aug 14, 2023

pladaria Aug 14, 2023

atabel Aug 21, 2023

pladaria Aug 14, 2023

atabel Aug 21, 2023

atabel commented Aug 21, 2023

marcoskolodny left a comment •

edited

Loading

atabel commented Aug 22, 2023

yceballost commented Aug 22, 2023 •

edited

Loading

atabel commented Aug 22, 2023 •

edited

Loading

yceballost commented Aug 23, 2023 •

edited

Loading

yceballost left a comment

tuentisre commented Aug 24, 2023

feat(Sheet): new component #840

feat(Sheet): new component #840

Conversation

atabel commented Aug 3, 2023 • edited Loading

github-actions bot commented Aug 3, 2023 • edited Loading

github-actions bot commented Aug 3, 2023 • edited Loading

github-actions bot commented Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pladaria Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pladaria Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atabel commented Aug 21, 2023

marcoskolodny left a comment • edited Loading

Choose a reason for hiding this comment

atabel commented Aug 22, 2023

yceballost commented Aug 22, 2023 • edited Loading

atabel commented Aug 22, 2023 • edited Loading

yceballost commented Aug 23, 2023 • edited Loading

yceballost left a comment

Choose a reason for hiding this comment

tuentisre commented Aug 24, 2023

atabel commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

pladaria Aug 14, 2023 •

edited

Loading

pladaria Aug 14, 2023 •

edited

Loading

marcoskolodny left a comment •

edited

Loading

yceballost commented Aug 22, 2023 •

edited

Loading

atabel commented Aug 22, 2023 •

edited

Loading

yceballost commented Aug 23, 2023 •

edited

Loading