Skip to content

BottomSheetWithSearchInput component - #110

Open
barry-observation wants to merge 1 commit into
developfrom
feature/bottom-sheet-with-search-input
Open

BottomSheetWithSearchInput component#110
barry-observation wants to merge 1 commit into
developfrom
feature/bottom-sheet-with-search-input

Conversation

@barry-observation

Copy link
Copy Markdown
Contributor

Creates a BottomSheetWithSearchInput component in the library so we can share it across Observation and ObsIdentify.

Comment thread package.json
}
},
"resolutions": {
"react-is": "19.2.3"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dit bleek nodig te zijn om een rare fout in de tests op te lossen. Zit ook in de 2 apps. Ik kwam erop na het lezen van dit issue.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die was ook nodig in de apps, ik kan me nog herinneren dat dit een lastige was om te debuggen.

@SjaakSchilperoort

SjaakSchilperoort commented Jul 30, 2026

Copy link
Copy Markdown
Member

Prima, maar ik stel voor om deze te rebasen op branch feature/update-dependencies-2, #112 (of te wachten tot dit in develop zit), in plaats van te valideren tegen een al snel verouderde package.json.

@SjaakSchilperoort

Copy link
Copy Markdown
Member

Kan gerebased worden op develop, daarna zal ik hem checken.

@SjaakSchilperoort

Copy link
Copy Markdown
Member

Of zal ik ook het rebasen doen?

@barry-observation
barry-observation force-pushed the feature/bottom-sheet-with-search-input branch from 620d222 to 3c936bf Compare July 30, 2026 13:19
@barry-observation
barry-observation force-pushed the feature/bottom-sheet-with-search-input branch from 3c936bf to 2be7055 Compare July 30, 2026 13:24
@barry-observation

Copy link
Copy Markdown
Contributor Author

Ik heb net een rebase gedaan en de dependencies nog wat aangepast. Zowel react-native-gesture-handler als @gorhom/bottom-sheet zijn ook dependencies van de apps. Daarom heb ik ze allebei in peerDependencies gezet met een range en in devDependencies met een exacte versie. Hopelijk dat we daarmee geen conflicten hebben. Ben benieuwd of dit ook is wat jij zou hebben gedaan.

@SjaakSchilperoort SjaakSchilperoort left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let erop dat er nu niets wordt geëxporteerd, dus de apps kunnen dit niet gebruiken. Ik zou verwachten:

  • Export van BottomSheetWithSearchInput en useBottomSheetBackHandler in src/index.ts
  • Export van bottomSheet in src/styles/index.ts

Comment on lines +13 to +14
import { Icon, InputField, ItemSeparator, ListItem, SectionHeader } from '@observation.org/react-native-components'
import { type Theme, createInputStyles, useStyles, useTheme } from '@observation.org/react-native-components/theme'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De library krijgt zo een dependency op zichzelf. Dit kan gewoon een intern pad zijn. Hetzelfde geldt voor nog 5 source files, goed om meteen ook aan te passen.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deze file kan weg


import mockBottomSheet from '@gorhom/bottom-sheet/mock'

class MockBottomSheetModal extends React.Component {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wordt deze mock ergens gebruikt? Zo te zien niet, en dan kan ie weg.

Comment thread package.json
"devDependencies": {
"@eslint/eslintrc": "^3.3.6",
"@eslint/js": "^9.39.5",
"@gorhom/bottom-sheet": ">=5.2.14",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dit is te open, in devDependencies zetten we de major version vast:

Suggested change
"@gorhom/bottom-sheet": ">=5.2.14",
"@gorhom/bottom-sheet": "^5.2.14",

onChangeText: onChange,
}}
rightIcon={
<TouchableOpacity

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatief is om in de componenten-library een BottomSheetIconButton toe te voegen.

@SjaakSchilperoort

Copy link
Copy Markdown
Member

@barry-observation Ik stel voor dat je ook de app-aanpassingen doet om de component met fix te gebruiken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants