Skip to content

Conversation

@marcosrdz
Copy link
Member

@marcosrdz marcosrdz commented Jun 1, 2025

Use builtin detents in RNav 7 isntead of using a dep package
@Overtorment
Copy link
Member

built in what..?
did you remove the dep?

@marcosrdz
Copy link
Member Author

built in what..?
did you remove the dep?

Sheetdetents. Basically builtin modals.

Im gonna remove the dep once all modals use the builtin mechanism. Otherwise the PR will be far bigger

@Overtorment
Copy link
Member

e2e failed

@marcosrdz
Copy link
Member Author

e2e failed

Fixed

@marcosrdz marcosrdz self-assigned this Jun 3, 2025
@limpbrains
Copy link
Collaborator

limpbrains commented Jun 3, 2025

Not great not terrible

@limpbrains
Copy link
Collaborator

limpbrains commented Jun 3, 2025

Not great not terrible

can we do better on android ?

@limpbrains
Copy link
Collaborator

If the issue with keyboard covering the form, I think we can do [0.4, 0.9], looks better
And wait for this to be merged

fee.webm

name="SelectFee"
component={SelectFeeScreen}
options={navigationStyle({
sheetAllowedDetents: Platform.OS === 'ios' ? 'fitToContents' : [0.9],
Copy link
Collaborator

Choose a reason for hiding this comment

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

why revert ?

Copy link
Member Author

Choose a reason for hiding this comment

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

E2e failed.
Gotta check why

Copy link
Member

@Overtorment Overtorment left a comment

Choose a reason for hiding this comment

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

lgtm

@limpbrains whats the verdict? any changes necessary?

@Overtorment Overtorment requested a review from limpbrains June 10, 2025 08:34
@GladosBlueWallet
Copy link
Collaborator

@limpbrains
Copy link
Collaborator

we can adjust sheetAllowedDetents for android later.
IMHO it should be dynamic.
e2e failed because 0.4 is not enought for small screens

testDone

@GladosBlueWallet GladosBlueWallet merged commit af55943 into master Jun 10, 2025
13 checks passed
@GladosBlueWallet
Copy link
Collaborator

Unbelievable. You, [subject name here], must be the pride of [subject hometown here]!

@GladosBlueWallet GladosBlueWallet deleted the formmod branch June 10, 2025 10:41
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.

5 participants