Skip to content

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

Merged
ashiagr merged 14 commits intodevelopfrom
issue/13268-introduce-date-range-picker-screen
Nov 18, 2020
Merged

Issue/13268 introduce date range picker screen in Activity Log#13324
ashiagr merged 14 commits intodevelopfrom
issue/13268-introduce-date-range-picker-screen

Conversation

@malinajirka
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka commented Nov 9, 2020 •
edited
Loading

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.

@malinajirka malinajirka added this to the 16.2 milestone Nov 9, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 9, 2020 •
edited
Loading

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

@malinajirka malinajirka mentioned this pull request Nov 9, 2020
Closed
31 tasks
@malinajirka malinajirka marked this pull request as draft November 9, 2020 10:01
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 9, 2020 •
edited
Loading

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
ParaskP7
ParaskP7 reviewed Nov 10, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

malinajirka commented Nov 10, 2020 •
edited
Loading

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 ashiagr self-assigned this Nov 16, 2020
@ashiagr
Copy link
Copy Markdown
Contributor

ashiagr commented Nov 16, 2020 •
edited
Loading

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

@malinajirka
Fix MaterialDatePicker confirm button after config change
29b9c17
@malinajirka
Use child fragment manager for date range picker fragment
b7c44fe
@malinajirka
Merge branch 'develop' into issue/13268-introduce-date-range-picker-s…
406d82e 
…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
Copy Markdown
Contributor Author

malinajirka commented Nov 17, 2020

Nice catch @ashiagr! Thanks for the review ;)

Fixed in 29b9c17 and b7c44fe.

ashiagr
ashiagr approved these changes Nov 18, 2020
View reviewed changes
Copy link
Copy Markdown
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

Reviewers

@ParaskP7 ParaskP7 ParaskP7 left review comments

@zwarm zwarm Awaiting requested review from zwarm

+1 more reviewer

@ashiagr ashiagr ashiagr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ashiagr ashiagr

Labels

Activity Log Jetpack

Projects

None yet

Milestone

16.3

Development

Successfully merging this pull request may close these issues.

Jetpack Section: Activity Log

3 participants

@malinajirka @ashiagr @ParaskP7