Add UI for accelerometer calibration #2451
Merged
Conversation
This file contains 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
Williangalvani
force-pushed
the
accelcal
branch
2 times, most recently
from
d9d5b30
to
160eda0
Compare
Williangalvani
force-pushed
the
accelcal
branch
4 times, most recently
from
3a7b219
to
524c0bd
Compare
Williangalvani
force-pushed
the
accelcal
branch
2 times, most recently
from
d61ee23
to
a0f4b0f
Compare
Williangalvani
force-pushed
the
accelcal
branch
2 times, most recently
from
471136a
to
1739d25
Compare
Williangalvani
force-pushed
the
accelcal
branch
from
1739d25
to
480320b
Compare
Williangalvani
requested review from
patrickelectric,
joaoantoniocardoso and
JoaoMario109
patrickelectric
approved these changes
Mar 28, 2024
There was a problem hiding this comment.
Lol, Patrick merged before I finished the review, but:
- Some small UI problems:
- The dialogue titles are hard to read in dark mode (and just ugly anyway)
- (Maybe) the fonts are smaller than they should be for a dialogue?
- Quick Calibration works fine.
- Full calibration:
- It doesn't show a success message
- It doesn't let me close the dialogue
| </template> | ||
|
|
||
| <v-card> | ||
| <v-card-title class="text-h5 grey lighten-2"> |
There was a problem hiding this comment.
The light grey color makes it almost unreadable in dark mode because the font is white.
| </v-btn> | ||
| </v-card-actions> | ||
| </v-card> | ||
| </v-dialog> |
There was a problem hiding this comment.
Differently from the Quick Calibration, I can't close this dialogue.
|
Another one:
|
| }, | ||
| current_state_text() { | ||
| const AccelStateText: { [key: number]: string } = { | ||
| [CalState.WAITING_FOR_LEVEL_POSITION]: 'Place the vehicle on a level surface', |
There was a problem hiding this comment.
The message isn't clear enough: we must say what position we expect the vehicle to be. A level surface can mean anything.
| <div> | ||
| Quick calibration requires that you place the vehicle on a level surface and keep it still for a few seconds. | ||
| It is usually enough if your vehicle is generally leveled while in use. | ||
| </div> |
There was a problem hiding this comment.
why do we need a div here?
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.
depends on #2463