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.

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

Issue/13268 introduce date range picker screen in Activity Log #13324

Merged
merged 14 commits into from
Nov 18, 2020

Conversation

malinajirka
Copy link
Contributor

@malinajirka malinajirka commented Nov 9, 2020

Partially fixes #13268

Merge instructions:

  1. Review this PR
  2. Make sure Introduce activity log filters feature flag #13270 is merged
  3. Remove "Not ready for merge" label
  4. Merge this PR

This PR introduces Material Fullscreen Date Range picker component in Activity Log. It overrides some styles of the material component to facilitate future styling. The design of the date range picker hasn't been finalised yet so ignore the overall styling for now.

Unit tests will be added in a separate PR.

To test:

  1. My Site -> Avatar icon in the toolbar -> App Settings -> Test feature configuration
  2. Enable ActivityLogFiltersFeatureConfig and click on "Restart app"
  3. Open Activity Log
  4. Click on the "Date range" chip
  5. Notice the fullscreen date range picker is shown
  6. Select a date range
  7. Click on "Cancel"
  8. Click on the "Date range" chip and notice no date range is pre-selected
  9. Select a date range
  10. Click on "Save"
  11. Click on the "Date range" chip and notice the date range from step 9 is pre-selected

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 9, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 9, 2020

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka malinajirka marked this pull request as ready for review November 9, 2020 10:25
Base automatically changed from issue/13268-introduce-feature-flag to develop November 10, 2020 04:17
Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

Cool stuff, now we have a material picker that we can play with! 🌟

PS: I didn't use emoji this time, but I did use a suffix on each of my comments to give you some context of what to expect (like Minor: Xyx or Idea: Xyz). Let me know your thoughts.

@malinajirka
Copy link
Contributor Author

malinajirka commented Nov 10, 2020

Thanks for the review @ParaskP7!

PS: I didn't use emoji this time, but I did use a suffix on each of my comments to give you some context of what to expect (like Minor: Xyx or Idea: Xyz). Let me know your thoughts.

I like it. We actually already use "NitPick:" prefix on minor comments - eg. this comment.

@malinajirka malinajirka modified the milestones: 16.2, 16.3 Nov 12, 2020
@ashiagr
Copy link
Contributor

ashiagr commented Nov 16, 2020

Material date range picker looks great! Good job 👍.
Just noticed one issue when the last selected date range was not pre-selected the next time.

Steps to reproduce:

  • Open the date picker
  • Change orientation
  • Select a date range
  • Click save
  • Reopen the date picker
  • Notice that the last selected date range is not pre-selected

date-range

…creen

# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/ActivityLogListFragment.kt
#	WordPress/src/main/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModel.kt
@malinajirka
Copy link
Contributor Author

Nice catch @ashiagr! Thanks for the review ;)

Fixed in 29b9c17 and b7c44fe.

Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

LGTM 👍. I've tested and confirm that last selected data range is saved properly after save action is performed after orientation change.

@ashiagr ashiagr merged commit e54a293 into develop Nov 18, 2020
@ashiagr ashiagr deleted the issue/13268-introduce-date-range-picker-screen branch November 18, 2020 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jetpack Section: Activity Log
3 participants