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

Trace location organiser list (EXPOSUREAPP-5466) #2679

Merged
merged 21 commits into from
Mar 29, 2021
Merged

Trace location organiser list (EXPOSUREAPP-5466) #2679

merged 21 commits into from
Mar 29, 2021

Conversation

AlexanderAlferov
Copy link
Contributor

@AlexanderAlferov AlexanderAlferov commented Mar 23, 2021 •
edited
Loading

Trace locations organiser list

screenshot-1616502472336

How to test:

  1. Open Test Menu
  2. Go to Event Registration fragment
  3. In Events section
  4. Show Organiser List - to show fragment
  5. Generate test race locations - to create test trace locations

@AlexanderAlferov
Initial commit
1d1abd3
@AlexanderAlferov
Merge remote-tracking branch 'origin/release/2.0.x' into feature/5466…
bcf9690 
…-trace-location-organiser-qr-codes-list

# Conflicts:
#	Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/menu/ui/TestMenuFragmentViewModel.kt
#	Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/ui/main/MainActivityTestModule.kt
#	Corona-Warn-App/src/deviceForTesters/res/navigation/test_nav_graph.xml
#	Corona-Warn-App/src/main/res/layout/trace_location_organiser_qr_codes_list_item.xml
#	Corona-Warn-App/src/main/res/values/strings.xml
@AlexanderAlferov
Button added
9ec3ace
@AlexanderAlferov
Streamlined list
8f1e8f7
@AlexanderAlferov
Merge remote-tracking branch 'origin/release/2.0.x' into feature/5466…
a1e4aff 
…-trace-location-organiser-qr-codes-list

# Conflicts:
#	Corona-Warn-App/src/deviceForTesters/res/navigation/test_nav_graph.xml
#	Corona-Warn-App/src/main/res/drawable/ic_baseline_more_vert_24.xml
#	Corona-Warn-App/src/main/res/values/strings.xml
@AlexanderAlferov
Merge remote-tracking branch 'origin/release/2.0.x' into feature/5466…
5cd7710 
…-trace-location-organiser-qr-codes-list
@AlexanderAlferov
Complete test fragment
8ba3e36
@AlexanderAlferov
Merge remote-tracking branch 'origin/release/2.0.x' into feature/5466…
a54074a 
…-trace-location-organiser-qr-codes-list

# Conflicts:
#	Corona-Warn-App/src/main/res/values-de/event_registration_strings.xml
#	Corona-Warn-App/src/main/res/values/event_registration_strings.xml
@AlexanderAlferov
Moved to clean
adb795b
@AlexanderAlferov
Clean up
36605d5
@AlexanderAlferov AlexanderAlferov added maintainers Tag pull requests created by maintainers text change PRs with text changes. labels Mar 23, 2021
@AlexanderAlferov AlexanderAlferov added this to the 2.0.0 milestone Mar 23, 2021
@AlexanderAlferov AlexanderAlferov requested a review from d4rken March 23, 2021 14:40
@AlexanderAlferov AlexanderAlferov marked this pull request as ready for review March 23, 2021 14:50
@AlexanderAlferov AlexanderAlferov requested review from a team March 23, 2021 14:50
mtwalli
mtwalli reviewed Mar 23, 2021
View reviewed changes
Copy link
Contributor

@mtwalli mtwalli left a comment

Choose a reason for hiding this comment

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

Amazing work 🤩, just small stuff

Corona-Warn-App/src/main/res/layout/trace_location_organizer_trace_locations_list_fragment.xml Outdated Show resolved Hide resolved
Corona-Warn-App/src/main/res/layout/trace_location_organizer_trace_locations_item.xml Show resolved Hide resolved
Corona-Warn-App/src/main/res/layout/trace_location_organizer_trace_locations_list_fragment.xml Outdated Show resolved Hide resolved
...c/deviceForTesters/java/de/rki/coronawarnapp/test/organiser/ui/TestTraceLocationsFragment.kt Outdated Show resolved Hide resolved
...main/java/de/rki/coronawarnapp/ui/eventregistration/organizer/list/TraceLocationsFragment.kt Show resolved Hide resolved
@mtwalli mtwalli self-assigned this Mar 23, 2021
@jurajkusnier jurajkusnier self-assigned this Mar 24, 2021
@jurajkusnier
Copy link
Contributor

Why do you need almost identical fragments and viewmodels?
TraceLocationsViewModel
TraceLocationsFragment
TestTraceLocationsViewModel
TestTraceLocationsFragment
That could be error-prone with a lot of duplications. You can extract generateTestData() method from TestTraceLocationsViewModel as new button in Test Menu > Event Registration and the rest could be in one fragment and one viewmodel.

@mtwalli
Copy link
Contributor

mtwalli commented Mar 25, 2021

Maybe in a follow-up PR you can implement Swipe-To-Delete after #2684 is merged

@ralfgehrer ralfgehrer added the prio PRs to review first. label Mar 29, 2021
mtwalli
mtwalli previously approved these changes Mar 29, 2021
View reviewed changes
Copy link
Contributor

@mtwalli mtwalli left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

jurajkusnier
jurajkusnier previously approved these changes Mar 29, 2021
View reviewed changes
janetback
janetback previously approved these changes Mar 29, 2021
View reviewed changes
Copy link
Contributor

@janetback janetback left a comment

Choose a reason for hiding this comment

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

UA reviewed.

@AlexanderAlferov
Merge remote-tracking branch 'origin/release/2.0.x' into feature/5466…
a40403b 
…-trace-location-organizer-list

# Conflicts:
#	Corona-Warn-App/src/deviceForTesters/res/navigation/test_nav_graph.xml
#	Corona-Warn-App/src/main/res/values-de/event_registration_strings.xml
#	Corona-Warn-App/src/main/res/values/event_registration_strings.xml
@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2021

Kudos, SonarCloud Quality Gate passed!

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

janetback
janetback reviewed Mar 29, 2021
View reviewed changes
Copy link
Contributor

@janetback janetback left a comment

Choose a reason for hiding this comment

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

UA reviewed.

axelherbstreith
axelherbstreith reviewed Mar 29, 2021
View reviewed changes
Corona-Warn-App/src/main/res/layout/trace_location_organizer_trace_locations_item.xml
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/icon"
app:layout_constraintTop_toBottomOf="@id/address"
app:layout_constraintVertical_bias="0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

app:layout_constraintVertical_bias="0.0" can be removed

@axelherbstreith
Copy link
Contributor

Looks really good 👍

One small remark, nothing blocking. When scrolling to the bottom, the collapsed floating action button is overlaying the "Check out" button of the last item. Maybe we can set the clipToPadding attribute to solve this. But this can be done in a separate PR

janetback
janetback approved these changes Mar 29, 2021
View reviewed changes
Copy link
Contributor

@janetback janetback left a comment

Choose a reason for hiding this comment

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

UA approved.

@AlexanderAlferov AlexanderAlferov merged commit 3e5df5b into release/2.0.x Mar 29, 2021
@AlexanderAlferov AlexanderAlferov deleted the feature/5466-trace-location-organizer-list branch March 29, 2021 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@axelherbstreith axelherbstreith axelherbstreith left review comments

@janetback janetback janetback approved these changes

@mtwalli mtwalli mtwalli approved these changes

@jurajkusnier jurajkusnier jurajkusnier approved these changes

@d4rken d4rken Awaiting requested review from d4rken

Assignees

@jurajkusnier jurajkusnier

@mtwalli mtwalli

Labels
maintainers Tag pull requests created by maintainers prio PRs to review first. text change PRs with text changes.
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@AlexanderAlferov @jurajkusnier @mtwalli @axelherbstreith @janetback @ralfgehrer @harambasicluka