Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove fragment integration point #147

Merged
merged 4 commits into from
Sep 12, 2022
Merged

Remove fragment integration point #147

merged 4 commits into from
Sep 12, 2022
+2 −51

Conversation

KovalevAndrey
Copy link
Collaborator

@KovalevAndrey KovalevAndrey commented Sep 12, 2022 •
edited
Loading

Description

Fixes #146

Check list

  • I have updated CHANGELOG.md if required.
  • I have updated documentation if required.

@KovalevAndrey KovalevAndrey force-pushed the remove-fragment-integration-point branch from 29f23b2 to 88f51c6 Compare September 12, 2022 09:38
@KovalevAndrey KovalevAndrey force-pushed the remove-fragment-integration-point branch from 88f51c6 to 0f2fdf2 Compare September 12, 2022 09:39
@KovalevAndrey KovalevAndrey added the cleanup Something that's no longer needed and should be removed label Sep 12, 2022
@KovalevAndrey KovalevAndrey self-assigned this Sep 12, 2022
@LachlanMcKee
Copy link
Collaborator

Are you sure that this will work as expected? There is some logic in this integration point about navigating up etc that. Perhaps removing this is good as it leaves that decision up to users?

@KovalevAndrey
Copy link
Collaborator Author

Are you sure that this will work as expected? There is some logic in this integration point about navigating up etc that. Perhaps removing this is good as it leaves that decision up to users?

FragmentIntegrationPoint effectively does the same as ActivityIntegrationPoint but call calling val activity = fragment.requireActivity() before action so I don't change the behaviour here

LachlanMcKee
LachlanMcKee approved these changes Sep 12, 2022
View reviewed changes
Copy link
Collaborator

@LachlanMcKee LachlanMcKee left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification

@KovalevAndrey KovalevAndrey merged commit 0699df6 into bumble-tech:main Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zsoltk zsoltk zsoltk approved these changes

@LachlanMcKee LachlanMcKee LachlanMcKee approved these changes

@CherryPerry CherryPerry CherryPerry approved these changes

Assignees

@KovalevAndrey KovalevAndrey

Labels
cleanup Something that's no longer needed and should be removed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove FragmentIntegrationPoint
4 participants
@KovalevAndrey @LachlanMcKee @zsoltk @CherryPerry