-
Notifications
You must be signed in to change notification settings - Fork 817
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
fix: gyroscope screen crash and UI #2651
base: development
Are you sure you want to change the base?
fix: gyroscope screen crash and UI #2651
Conversation
Reviewer's Guide by SourceryThis pull request addresses a crash in the gyroscope screen and improves UI visibility in landscape mode. It adds checks to ensure fragments are attached before accessing context, uses safe context access for string resources, and uses ScrollView to ensure proper UI visibility in landscape mode. Sequence diagram for visualizeData() methodsequenceDiagram
participant GDF as GyroscopeDataFragment
participant GV as GyroscopeViewFragment
GDF->GDF: isAdded()
alt Fragment is attached
loop for each gyroscopeViewFragment
GDF->GV: fragment.isAdded()
alt Fragment is attached
GDF->GDF: timeElapsed = (System.currentTimeMillis() - startTime) / 1000
GDF->GV: setPreviousTimeElapsed(timeElapsed)
GDF->GV: addEntry(new Entry((float) timeElapsed, fragment.getCurrentValue()))
GDF->GV: getContext()
alt Context is not null
GDF->GV: getString(R.string.gyroscope)
else Context is null
GDF->GDF: label = "Gyroscope"
end
GDF->GV: LineDataSet dataSet = new LineDataSet(fragment.getEntries(), label)
end
end
end
Updated class diagram for GyroscopeDataFragmentclassDiagram
class GyroscopeDataFragment {
-List~GyroscopeViewFragment~ gyroscopeViewFragments
-long startTime
+void visualizeData()
+void writeLogToFile(long timestamp, float readingX, float readingY, float readingZ)
}
GyroscopeDataFragment -- GyroscopeViewFragment : has a
Updated class diagram for fragment_gyroscope_data.xmlclassDiagram
class ScrollView {
---
}
class LinearLayout{
---
}
class Fragment{
---
}
ScrollView o-- LinearLayout : contains
LinearLayout o-- Fragment : contains
LinearLayout o-- Fragment : contains
LinearLayout o-- Fragment : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @rahul31124 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The addition of
isAdded()
checks is good for preventing crashes, but consider whether there are alternative ways to handle the lifecycle of these fragments. - Using hardcoded values like
android:layout_height="300dp"
should be avoided; consider using a dimension resource instead.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/13923861898/artifacts/2772510925 |
b37abab
to
014a6a9
Compare
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.
@rahul31124 Thanks for your work. Could you please fix all the hardcoded dimensions ?
Hi @AsCress , I have fixed all the hardcoded dimensions now I guess everything looks good |
Hardcoded dimensions
fc11bfb
to
20db51d
Compare
Bug Fixes
The gyroscope screen was crashing and the gyroscope screen ui was not properly visible in landscape mode
Exception
java.lang.IllegalStateException: Fragment GyroscopeDataFragment{9c6c7cf} (e25b37d6-0238-4496-aaa5-74f0d1aa9d93) not attached to a context.
Changes
Added checks to ensure fragments are properly attached before accessing context (isAdded() and fragment.isAdded() checks)
Used safe context access for string resources with a fallback string
Used ScrollView to ensure proper UI visibility in landscape mode
Screenshots / Recordings
Before
Gyroscope_Before.mp4
After
Gyroscope_After.mp4
Checklist:
strings.xml
,dimens.xml
andcolors.xml
without hard coding any value.strings.xml
,dimens.xml
orcolors.xml
.Summary by Sourcery
Fixes a crash on the gyroscope screen and improves UI visibility in landscape mode. The fragment is now properly attached before accessing the context and a ScrollView is used to ensure proper UI visibility.
Bug Fixes:
Enhancements: