-
-
Notifications
You must be signed in to change notification settings - Fork 128
feat: KeyboardToolbar.Exclude
#881
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
base: main
Are you sure you want to change the base?
Conversation
0c409b6
to
cf36b65
Compare
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
📊 Package size report
|
A slightly better approach would be to introduce a component: <KeyboardToolbar.Group name="group1" discoverable>
</KeyboardToolbar.Group> In this what effectively we would do is to split inputs in group under their own names. Such approach is more flexible in my inderstanding, because if we would have tab-view written in pure JS:
Then we in last input of tab 1 we could easily go to the second tab. If we would use I think a new API should offer better extendability for future goals. |
if (isValidTextInput(child)) { | ||
result = child as EditText | ||
} else if (child is ViewGroup) { | ||
} else if (child is ViewGroup && child !is KeyboardToolbarExcludeReactViewGroup) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check repeats twice, would it be better to use helper function for this?
return validTextInput | ||
} | ||
|
||
guard String(describing: type(of: view)) != "KeyboardToolbarExcludeView" else { return nil } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- is it possible to use instance check instead of comparing two strings?
- it repeats twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 16 changed files in this pull request and generated 1 comment.
Files not reviewed (10)
- android/src/base/java/com/reactnativekeyboardcontroller/KeyboardControllerPackage.kt: Language not supported
- android/src/main/java/com/reactnativekeyboardcontroller/managers/KeyboardToolbarExcludeViewManagerImpl.kt: Language not supported
- android/src/main/java/com/reactnativekeyboardcontroller/traversal/ViewHierarchyNavigator.kt: Language not supported
- android/src/main/java/com/reactnativekeyboardcontroller/views/KeyboardToolbarExcludeReactViewGroup.kt: Language not supported
- android/src/paper/java/com/reactnativekeyboardcontroller/KeyboardToolbarExcludeViewManager.kt: Language not supported
- android/src/turbo/java/com/reactnativekeyboardcontroller/KeyboardControllerPackage.kt: Language not supported
- docs/docs/api/components/keyboard-toolbar/index.mdx: Language not supported
- ios/traversal/ViewHierarchyNavigator.swift: Language not supported
- ios/views/KeyboardToolbarExcludeViewManager.h: Language not supported
- ios/views/KeyboardToolbarExcludeViewManager.mm: Language not supported
: ({ children }: KeyboardGestureAreaProps) => children; | ||
export const RCTOverKeyboardView: React.FC<OverKeyboardViewProps> = | ||
require("./specs/OverKeyboardViewNativeComponent").default; | ||
export const RCTKeyboardToolbarExcludeView: React.FC<OverKeyboardViewProps> = |
Copilot
AI
Apr 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type for RCTKeyboardToolbarExcludeView should use KeyboardToolbarExcludeViewProps instead of OverKeyboardViewProps for consistency with the rest of the bindings.
export const RCTKeyboardToolbarExcludeView: React.FC<OverKeyboardViewProps> = | |
export const RCTKeyboardToolbarExcludeView: React.FC<KeyboardToolbarExcludeViewProps> = |
Copilot uses AI. Check for mistakes.
## 📜 Description Added compound pattern to `KeyboardToolbar` component. ## 💡 Motivation and Context This component started as a very simple component with limited customization. However, over the time it got many new props, such as `onNextCallback` etc. and right now we have too many props for customization. Since I'm planning to add #881 I decided that it'll be a good time to refactor the component and apply compound pattern. So in the end I come up with next API design: ```tsx <KeyboardToolbar> <KeyboardToolbar.Background /> <KeyboardToolbar.Prev /> <KeyboardToolbar.Next /> <KeyboardToolbar.Content /> <KeyboardToolbar.Done /> </KeyboardToolbar> ``` > I intentionally changed `blur` prop to `background`, because with iOS 26 we may use liquid glass as a container Closes #1121 ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### JS - added new compound `KeyboardToolbar` API; - moved props declaration into `types.ts` file. ### Docs - added new API reference; - added migration guide. ## 🤔 How Has This Been Tested? - tested that old code works with new API and doesn't introduce visual regressions (via e2e tests) - started to use new API and assure that it doesn't break e2e tests. ## 📸 Screenshots (if appropriate): <img width="614" height="422" alt="image" src="https://github.com/user-attachments/assets/8d2cc932-7fd8-4f29-89d4-bf4961bd97dc" /> ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
📜 Description
💡 Motivation and Context
Closes #470
📢 Changelog
JS
iOS
Android
🤔 How Has This Been Tested?
📸 Screenshots (if appropriate):
📝 Checklist