Repository navigation
Slider custom fields - #104
Merged
Merged
Conversation
…an error The server and the web app gained a slider field type; this is the Android side of it. Two kinds, as the server defines them: labeled, a fixed 0 to 100 between two words that reads as a percentage, and numeric, a quantity in the owner's own range with a step and a unit. Either can paint a gradient across the track. Settings gets the full setup with a live preview, the member editor gets the control, and a member's profile shows the reading. The reading is produced the same way the server produces it for a public profile, so a slider reads the same string wherever it appears. Three things worth saying about the shape of it: A slider's options are the one body this client sends that the server REPLACES rather than merges. A key left out is a key reset to its default, which is how a label, a unit or a gradient is cleared, since there is no explicit null to send. That makes Moshi's omit-nulls exactly right here and makes it the opposite of the PATCH bodies next door, so the wire shape is pinned by tests at the JSON rather than at the data class: a stray `choices` riding along on a slider would 422 the whole save. The track is drawn rather than left to Material. A gradient cannot be expressed as a track colour, so a gradient slider would otherwise be a different shape from one without, and the two sit next to each other in the same member's fields. Material's slider is Float-valued, and dragging to the 85 stop on a 0 to 100 track stored 85.00000762939453 while the reading said "85%". Values now snap to the step grid, or to the two decimals the reading shows, so the number stored is the number the user was shown. Separately, a field whose type the server knows and this build does not showed "Unknown field type" in red where its value should be, hiding what a member had put there behind something that looked like a fault. The API asks clients to treat an unknown type as text, so it now shows the value with a note to update the app. That is also why a slider field on an older build looks broken rather than merely unsupported. Verified against a server with the type: creating a labeled slider from the phone stores the canonical options, renaming the field leaves its shape intact, clearing a label and the gradient clears both and leaves the other label alone, and a dragged value lands as a clean number.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the slider custom field type across from the server and the web app, and fixes the way this client treats a field type it does not recognise.
Slider fields
Two kinds, as the server defines them:
Either can paint a gradient across the track, picked from the same nine named ones the web card offers or built from your own colours using the colour picker the app already uses for member and tag colours.
Settings gets the whole setup with a live preview. The member editor gets a draggable track with the reading beside it and a tap to clear. A member's profile shows the reading. The reading is produced the same way the server produces it for a public profile, so a slider reads the same string wherever it turns up.
Three things worth flagging in review
Options are replaced, not merged. A slider's options are the one body this client sends that the server rebuilds from scratch rather than patching. A key left out is a key reset to its default, which is how a label, a unit or a gradient gets cleared, since there is no explicit null to send. That makes Moshi's omit-nulls exactly right here and makes it the opposite of the PATCH bodies next door. Because the difference is invisible at the data class, the wire shape is pinned at the JSON instead: a stray
choicesriding along on a slider would 422 the whole save.The track is drawn rather than left to Material. A gradient cannot be expressed as a track colour, so a gradient slider would otherwise be a different shape from one without, and the two sit next to each other in the same member's field list. This was visibly wrong on device before the fix. Same bar either way now, and the same bar the read-only readout and the settings preview draw.
Float noise no longer reaches stored values. Material's slider is Float-valued, and dragging to the 85 stop on a 0 to 100 track stored
85.00000762939453while the reading said "85%". Values now snap to the step grid, or to the two decimals the reading shows, so the number stored is the number the user was shown. The web stores a clean85, so this was a real divergence.Unknown field types
A field whose type the server knows and this build does not showed "Unknown field type" in red where its value should be. That hid what a member had actually put there behind something that looks like a fault, and it is why a slider field on an older build looks broken rather than merely unsupported.
The API asks clients to treat an unknown type as text, so it now shows the value with a note to update the app. No editor is offered, deliberately: guessing at one risks writing a string into a field the server validates as something else.
Note this differs from the web, which falls through to a plain text input for a type it does not know. Happy to match that instead if you would rather the two agree, but read-only looked like the safer of the two for a client that cannot know the value's shape.
Also in here
The field type names on this screen now match the web card's ("Yes/No", "Select (single)", "Multi-select") rather than capitalising the API value, so the list, the dropdown and the edit dialog agree with the other client. Small user-visible change beyond the slider work, so calling it out.
Verification
Tests: 25 new, covering the options contract, the readings, the defensive resolution of a partial options object, the wire shape in both directions, which types may carry options, the bounds the server would refuse, gradient preset matching, and the float-noise regression with the exact value observed on device. I checked the wire test fails when the labeled/numeric split is removed.
Checked against a server with the type, from the phone: