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

Fix Crash when Rotating from Portrait to Landscape Mode #30

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Fix Crash when Rotating from Portrait to Landscape Mode #30

merged 4 commits into from
Dec 7, 2023

Conversation

wba2hi
Copy link
Contributor

@wba2hi wba2hi commented Dec 5, 2023

Closes: #26

wba2hi added 3 commits December 5, 2023 09:05
@wba2hi
feature: Add Dependency Injection using Hilt
f96062a 
Signed-off-by: Andre Weber <andre.weber3@etas.com>
@wba2hi
fix: Crash when Rotating from Portrait to Landscape Mode
1624717 
The App crashed due to the Android DataStore being re-instantiated
during the recreation of the Activity. This has been fixed by using
Hilt for Dependency Injection to create the DataStore as a Singleton.

Closes: eclipse-kuksa#26
Signed-off-by: Andre Weber <andre.weber3@etas.com>
@wba2hi
chore: Inject ConnectionInfoRepository into SettingsViewModel
fa49aae
@wba2hi
Copy link
Contributor Author

wba2hi commented Dec 5, 2023

Even though Hilt was added now let's always keep in mind not to "over-abuse" it.
When you have a hammer everything might look like a nail, but it isn't. Let's try to use dependency injection only where absolutely needed or helpful and let's try not to entangle Hilt now with every component we add, so that these components can easily be re-used without explicitely having to introduce Hilt as well.

lukasmittag
lukasmittag approved these changes Dec 5, 2023
View reviewed changes
Copy link

@lukasmittag lukasmittag left a comment

Choose a reason for hiding this comment

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

Works for me with emulated Pixel 3a!

Code looks good, at least I understood what is done (after googling some stuff). I think it's the best solution to #26

lukasmittag
lukasmittag reviewed Dec 5, 2023
View reviewed changes
app/build.gradle.kts
@@ -34,8 +36,8 @@ android {
applicationId = "org.eclipse.kuksa.companion"
minSdk = 27
targetSdk = 33
versionCode = 1
versionName = "1.0"
versionCode = rootProject.extra["projectVersionCode"].toString().toInt()
Copy link

Choose a reason for hiding this comment

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

this I could not check/didn't know how to check.

Copy link

Choose a reason for hiding this comment

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

found it in App Infos looks good :)

@wba2hi
Copy link
Contributor Author

wba2hi commented Dec 7, 2023

@SebastianSchildt
PR can be merged

@SebastianSchildt SebastianSchildt merged commit bc48cc1 into eclipse-kuksa:main Dec 7, 2023
5 checks passed
@erikbosch erikbosch deleted the feature-26 branch October 31, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukasmittag lukasmittag lukasmittag approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crashes when rotating from portrait to landscape mode
3 participants
@wba2hi @lukasmittag @SebastianSchildt