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

Add UI features and algorithm to support label inference #762

Merged
merged 3 commits into from
Jul 1, 2021

Conversation

GabrielKS
Copy link
Contributor

  • Phone now receives and processes label inference data structure from the server to determine what inferences to show the user
  • Makes labels red if they have no value, yellow if they are inferred, and green if they are user input
  • Adds a "verify" button that turns yellow labels green

+ Adds basic client-side processing of label inference data structure
    - Minor restructuring of infinite_scroll_list.js to facilitate this
    - Still need to write full client-side inference algorithm
+ Displays inferred labels in yellow and blank labels in red
+ Moves the "more" button on trip cards to the top right corner
+ Adds a verify button beneath it that turns yellow labels green
+ Fixes a bug that mis-positioned trip start and end icons if labels were blank
+ Like the previous commit, does not affect the Diary page
+ This algorithm takes the list of label tuples and probabilities from
  the server and chooses which yellow labels to show the user, taking
  into account what the user has already begun to enter.
+ Extensively documented in the comments.

Testing done:
Tested various cases using the more extensive server-side placeholder
label inference "algorithm" recently committed.
@shankari
Copy link
Contributor

@GabrielKS can you add some screenshots or animated gifs here to make it easier to review without pulling the changes and running them?

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward; I will merge after you upload some screenshots/animated gifs

</div>
</div>
<div ng-click="verifyTrip($event, trip)" class="col-10 diary-checkmark-container center-vert center-horiz">
<i ng-class="trip.verifiability" class="ion-checkmark-round"></i>
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, please continue using properties instead of functions here. Using functions to retrieve the ng-class, although it works, plays havoc with the caching and the performance really suffers.

@GabrielKS
Copy link
Contributor Author

Below is a GIF that showcases all of the changes implemented. The inference data is made up for instructional purposes.
Showcase GIF

Here's what is happening for each trip:

  • Trip 1: No inference possible. Same behavior as before these changes, except blank labels are red instead of gray.
  • Trip 2: Both labels can be inferred. Verify button marks labels as user-confirmed, turning them green.
  • Trip 3: Only mode can be inferred; verify button confirms it.
  • Trip 4: Displayed inferences are incorrect. User inputs mode, at which point the system can correctly infer purpose.
  • Trip 5: Only mode can be inferred with enough confidence to display it; once user confirms that, confidence in the purpose inference increases to the point that it can be displayed.
  • Trip 6: Only mode can be inferred with enough confidence to display it, but it is incorrect; user enters purpose, at which point the system can correctly infer mode.

@shankari
Copy link
Contributor

shankari commented Jul 1, 2021

Please submit the PR to fix the confirmation check for Trip 1 after I merge this.

@shankari shankari merged commit 4e1ac3a into e-mission:master Jul 1, 2021
@GabrielKS GabrielKS mentioned this pull request Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants