-
Notifications
You must be signed in to change notification settings - Fork 1
feat: ui material design overhaul (formerly bottom sheet material) #237
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
base: master
Are you sure you want to change the base?
Conversation
📊 Code Coverage Summary
|
9d50996 to
7ac3324
Compare
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
android/app/src/main/java/com/github/quarck/calnotify/ui/EditEventActivity.kt
Show resolved
Hide resolved
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
3 similar comments
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
| .show() | ||
| } | ||
|
|
||
| private fun localMidnightMillisFromUtcDateSelection(dateSelectionUtcMillis: Long): Long { |
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.
This should be in a utility function class somewhere and tested separately
| picker.show(supportFragmentManager, "timeFromPicker") | ||
| } | ||
|
|
||
| @Suppress("UNUSED_PARAMETER") |
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.
Can We get rid of this suppression?
| @@ -2,8 +2,8 @@ package com.github.quarck.calnotify.ui | |||
|
|
|||
| import android.view.LayoutInflater | |||
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.
Given that we've changed the UI here will need to pay close attention to how this test changes things and make sure it's testing the right stuff
| layoutCustomPattern = view.findOrThrow(R.id.layout_reminder_interval_custom) | ||
|
|
||
| timeUnitsSpinners.adapter = ArrayAdapter( | ||
| timeUnitsArray = view.context.resources.getStringArray(R.array.time_units_plurals_with_seconds) |
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.
What's this?
| if (simpleIntervalMillis == 0L) { | ||
| simpleIntervalMillis = 60 * 1000L | ||
| Toast.makeText(context, R.string.invalid_reminder_interval, Toast.LENGTH_LONG).show() | ||
| Toast.makeText(requireContext(), R.string.invalid_reminder_interval, Toast.LENGTH_LONG).show() |
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.
We should have some guidelines in the docs on where to and not to use toasts and snackbars
| var entry: DismissedEventAlertRecord? = null | ||
|
|
||
| var eventHolder: RelativeLayout? | ||
| var eventHolder: View? |
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.
Just curious why this change?
| undoLayout = itemView.find<ViewGroup?>(R.id.event_card_undo_layout) | ||
|
|
||
| compactViewContentLayout = itemView.find<RelativeLayout?>(R.id.compact_view_content_layout) | ||
| compactViewContentLayout = itemView.find<ViewGroup?>(R.id.compact_view_content_layout) |
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.
Why?
| @@ -20,11 +20,10 @@ | |||
| package com.github.quarck.calnotify.ui | |||
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.
Got to think deeply about this. One of the most important parts of the app need to make sure it does the right things
| @@ -20,7 +20,11 @@ | |||
|
|
|||
| package com.github.quarck.calnotify.ui | |||
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.
Need a summary of the changes here. What are they? Why are we making them?
|
|
||
| import android.annotation.SuppressLint | ||
| import android.app.AlertDialog | ||
| import com.google.android.material.dialog.MaterialAlertDialogBuilder |
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.
Need a summary of the changes here and why we are been making them. Also, it seems like there's a whole lot of overlap with what we are doing in the view event activity. Also, I wonder if we could refactor that to be shared somehow?
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| private fun showToast(messageResId: Int) { | ||
| Toast.makeText(requireContext(), messageResId, Toast.LENGTH_LONG).show() | ||
| view?.let { Snackbar.make(it, messageResId, Snackbar.LENGTH_LONG).show() } |
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.
Snackbar silently fails when fragment view unavailable
Low Severity
The showToast function was changed from using Toast.makeText(requireContext(), ...) to view?.let { Snackbar.make(it, ...) }. If view is null (e.g., fragment view destroyed during activity recreation while file picker is open), the Snackbar silently fails to show and users receive no feedback about export/import success or failure. The original Toast implementation would show messages regardless of fragment view state.
📊 Code Coverage Summary
|
1 similar comment
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
refs #15
Note
Modernizes UI with Material components and refactors time selection flows.
AlertDialog, platformDatePicker/TimePicker, andToastwithMaterialAlertDialogBuilder,MaterialDatePicker,MaterialTimePicker, andSnackbaracross activities/fragments (edit/view/snooze, main, settings, dismissed events, pre-action)MaterialToolbar,MaterialDivider, Material checkboxes/radio buttons/cards, exposed dropdown menus; remove legacy date/time picker layoutsAutoCompleteTextView(exposed dropdown) instead ofSpinner, updating logic inTimeIntervalPickerControllerandReminderPatternPreferenceXR.string.choose_date; mark one flaky calendar monitor test as@IgnoreView/ViewGroupvsRelativeLayout) and addcalendar_color_indicatordrawableWritten by Cursor Bugbot for commit 4b80163. This will update automatically on new commits. Configure here.