Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Generic duration picker (DEV) #2546

Merged
merged 4 commits into from
Mar 12, 2021
Merged

Generic duration picker (DEV) #2546

merged 4 commits into from
Mar 12, 2021

Conversation

mtwalli
Copy link
Contributor

@mtwalli mtwalli commented Mar 8, 2021

Duration is picker is required for event registration too. This PR make the current implementation more generic and customisable using a builder pattern like Material way.

  • Rename the current picker to just DurationPicker,moved it a durationpicker package
  • Add Builder pattern to easy add more options for the dialog if wanted (such as title for example)
  • Hide args and dialog in creation in Builder

Testing

  • Check in the contact-diary and from Test menu

@mtwalli mtwalli added enhancement Improvement of an existing feature maintainers Tag pull requests created by maintainers labels Mar 8, 2021
@mtwalli mtwalli added this to the 1.15.0 milestone Mar 8, 2021
@mtwalli mtwalli requested a review from a team March 8, 2021 08:22
@harambasicluka harambasicluka self-assigned this Mar 8, 2021
@harambasicluka
Copy link
Contributor

@mtwalli Would you mind changing the base to 1.16.x? It's not required for 1.15.x and we still have a lot of PRs to go :)

@mtwalli mtwalli changed the base branch from release/1.15.x to release/1.16.x March 8, 2021 14:13
@mtwalli mtwalli requested a review from a team March 8, 2021 14:13
@mtwalli mtwalli force-pushed the dev/generic_duration_picker branch from e73dd54 to 937e49b Compare March 8, 2021 14:15
@mtwalli mtwalli marked this pull request as draft March 8, 2021 14:17
@ralfgehrer ralfgehrer modified the milestones: 1.15.0, 1.16.0 Mar 8, 2021
@d4rken d4rken changed the base branch from release/1.16.x to release/2.0.x March 9, 2021 16:13
@mtwalli
Make DurationPicker generic
f721828 
- Rename Picker to a more generic name
- Move it to a suitable package
- Avoid using Target API
- Implement Builder pattern
@mtwalli mtwalli force-pushed the dev/generic_duration_picker branch from 937e49b to f721828 Compare March 10, 2021 11:07
@mtwalli mtwalli marked this pull request as ready for review March 10, 2021 11:07
AlexanderAlferov
AlexanderAlferov approved these changes Mar 10, 2021
View reviewed changes
Copy link
Contributor

@AlexanderAlferov AlexanderAlferov left a comment

Choose a reason for hiding this comment

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

LGTM. Tested on Samsung S8.

@jurajkusnier jurajkusnier self-assigned this Mar 12, 2021
jurajkusnier
jurajkusnier approved these changes Mar 12, 2021
View reviewed changes
Copy link
Contributor

@jurajkusnier jurajkusnier left a comment

Choose a reason for hiding this comment

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

LGTM
Duration Picker will be updated for the purpose of Event Registration in another PR

@sonarcloud
Copy link

sonarcloud bot commented Mar 12, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

38.0% 38.0% Coverage
0.0% 0.0% Duplication

@harambasicluka harambasicluka merged commit c769c47 into release/2.0.x Mar 12, 2021
@harambasicluka harambasicluka deleted the dev/generic_duration_picker branch March 12, 2021 13:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AlexanderAlferov AlexanderAlferov AlexanderAlferov approved these changes

@jurajkusnier jurajkusnier jurajkusnier approved these changes

Assignees

@jurajkusnier jurajkusnier

@harambasicluka harambasicluka

@AlexanderAlferov AlexanderAlferov

Labels
enhancement Improvement of an existing feature maintainers Tag pull requests created by maintainers
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mtwalli @harambasicluka @jurajkusnier @AlexanderAlferov @ralfgehrer @d4rken