TheWidlarzGroup / react-native-notificated

⚑️React Native toast notifications
https://thewidlarzgroup.github.io/react-native-notificated/
MIT License
356 stars 18 forks source link

Better support for tablets #216

Closed eleddie closed 9 months ago

eleddie commented 1 year ago

Hi! πŸ‘‹

Firstly, thanks for your work on this project! πŸ™‚

I was facing an issue where the notifications in tablets are the size of the screen, so they are too big and don't look good.

This is a quick solution I found to have a better UI for notifications on tablets

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-notificated/src/core/config.ts b/node_modules/react-native-notificated/src/core/config.ts
index 954db1c..46f7105 100644
--- a/node_modules/react-native-notificated/src/core/config.ts
+++ b/node_modules/react-native-notificated/src/core/config.ts
@@ -7,6 +7,7 @@ const notificationSideMargin = 14
 const initialOffsetY = -300
 const targetOffsetY = true ? 50 : 10
 const maxNotificationWidth = 343
+const maxNotificationWidthTablet = 420

 export const Constants = {
   maxLongPressDragDistance,
@@ -15,4 +16,5 @@ export const Constants = {
   targetOffsetY,
   isAndroid,
   maxNotificationWidth,
+  maxNotificationWidthTablet
 }
diff --git a/node_modules/react-native-notificated/src/core/renderers/GestureHandler.tsx b/node_modules/react-native-notificated/src/core/renderers/GestureHandler.tsx
index 5c67815..20258d4 100644
--- a/node_modules/react-native-notificated/src/core/renderers/GestureHandler.tsx
+++ b/node_modules/react-native-notificated/src/core/renderers/GestureHandler.tsx
@@ -22,8 +22,9 @@ type Props = {

 export const GestureHandler = ({ children, state, animationAPI }: Props) => {
   const { width } = useWindowDimensions()
+
   const notificationWidth = state.isPortaitMode
-    ? width - Constants.notificationSideMargin * 2
+    ? Math.min(width, Constants.maxNotificationWidthTablet) - Constants.notificationSideMargin * 2
     : Constants.maxNotificationWidth
   return (
     <PanGestureHandler

This is how it looks with the current version of the library: Simulator Screen Shot - iPad Pro (12 9-inch) (6th generation) - 2023-07-19 at 18 37 05

It can be fixed by setting maxWidth in the custom notification: Simulator Screen Shot - iPad Pro (12 9-inch) (6th generation) - 2023-07-19 at 18 33 16

But the problem is that the PanResponder is still full width so you can't tap on the back button when the notification is shown: Simulator Screen Shot - iPad Pro (12 9-inch) (6th generation) - 2023-07-19 at 18 33 54

And this is how it looks after the change, you can tap on the back button without any issues: Simulator Screen Shot - iPad Pro (12 9-inch) (6th generation) - 2023-07-19 at 18 32 40

Okelm commented 1 year ago

@eleddie thanks for reporting the issue and proposing the solution!

PdoubleU commented 11 months ago

Hey @eleddie ,

Thanks for giving our library a spin and pointing out this important issue.

Your proposed solution looks really solid. I was wondering if it might be worth adding a new optional configuration element, let's name it notificationWidth, to give developers full control over the width.

Here's what I had in mind:

What do you think about this enhancement? Maybe you'd be interested in contributing to our library and preparing a PR with these changes?

Btw, looking at your screenshots, I understood that the library could offer more extensive configuration options for notificationPosition #218

eleddie commented 11 months ago

@PdoubleU sounds good! I'll work on it and open a PR once done. I'll tag you and @Okelm in itπŸ‘πŸ»

PdoubleU commented 9 months ago

@PdoubleU sounds good! I'll work on it and open a PR once done. I'll tag you and @Okelm in itπŸ‘πŸ»

Hi @eleddie, I just wanted to let you know that we are addressing this issue. Thanks for bringing it to our attention here :)