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

Tour Chart: Ability to show/hide the following data: Elevation, Distance, Duration, Elevation Gain Difference, Distance Difference, Duration Difference #300

Merged
merged 27 commits into from
Feb 15, 2021

Conversation

FJBDev
Copy link
Collaborator

@FJBDev FJBDev commented Feb 6, 2021

image

image

* Suunto App Integration

* Readme update

* Suunto App logo with transparent background

* Clean-up

* Renaming Cloud Connectivity to Cloud

* Info

* date

* Better naming

* renaming

* Streams!

* Better explanation

* Mnemonic

* Name

* Misc

* last cleanup

* Improvements

* Displaying a message when renewing the token
@wolfgang-ch
Copy link
Collaborator

image

I think the tooltip data fields should be displayed in 2 columns to keep this slideout in the width as small as possible and translations may need more width. When adding an additional field, then the width would also grow.

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 6, 2021

I think the tooltip data fields should be displayed in 2 columns to keep this slideout in the width as small as possible and translations may need more width. When adding an additional field, then the width would also grow.

I tried at first but it seemed to me that there was enough room for a third column.
But if you prefer 2 columns, I just fixed it:

image

@wolfgang-ch
Copy link
Collaborator

I think the tooltip data fields should be displayed in 2 columns to keep this slideout in the width as small as possible and translations may need more width. When adding an additional field, then the width would also grow.

I tried at first but it seemed to me that there was enough room for a third column.
But if you prefer 2 columns, I just fixed it:

image

With Linux there is a lot of empty space, mainly the spinner control width is about twice as on windows, therefore on Linux it do not look as good as on windows.

The controls could be better placed, delta distance is at the bottom left and distance is at the top right. I have put it in this order

elevation - elevation delta
distance - distance delta
time - time delta

Also the tooltip UI should have this layout without repeating the value name, just showing the delta char.

Is there a need that the user can select the displayed values or just show also the delta values?
Are you planning to show additional values?

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 8, 2021

The controls could be better placed, delta distance is at the bottom left and distance is at the top right. I have put it in this order

elevation - elevation delta
distance - distance delta
time - time delta

Fixed
image

Also the tooltip UI should have this layout without repeating the value name, just showing the delta char.

You mean something like this ?
Elevation 1720 m - Δ 270 m
Distance 6 km - Δ 1 km
Time 0:14h - Δ 0:07h

Is there a need that the user can select the displayed values or just show also the delta values?

I am not sure. I thought to make it configurable because maybe some users don't want the delta values ?
This is actually a request from Jesus

As for the parameters to be represented on the lap, I think it should
at least show also: lap time, lap distance, altitude gained on the lap.
I'm sure others will be missed, but I think these are essential. It
would be good if they were there and even better if you could choose
which ones. But of course, asking is very simple, doing it is more
complex and time consuming.

Jesús

Are you planning to show additional values?

Not at the moment

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 8, 2021

With Linux there is a lot of empty space, mainly the spinner control width is about twice as on windows, therefore on Linux it do not look as good as on windows.

Also, on Linux the label is not very readable as it is black and the text is grey
image

@wolfgang-ch
Copy link
Collaborator

wolfgang-ch commented Feb 15, 2021

With Linux there is a lot of empty space, mainly the spinner control width is about twice as on windows, therefore on Linux it do not look as good as on windows.

Also, on Linux the label is not very readable as it is black and the text is grey
image

fixed in a512832 the foreground color was not set, this was not an issue when using win10 with default theme

mt-tour-chart-marker-tooltip

@wolfgang-ch
Copy link
Collaborator

I would have tested but my oldest MT backup is from 19.10 (> 18.3)

The field renaming happend in // 41 -> 42 20.11.1

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 15, 2021

I would have tested but my oldest MT backup is from 19.10 (> 18.3)

The field renaming happend in // 41 -> 42 20.11.1

yes but didn't you tell that the database issue only happened with database versions < 18.13 ? "I wanted to run MT with data before 18.13.1 but it failed"

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 15, 2021

mt-tour-chart-marker-tooltip

Just tested and That's looking good on my system!

Copy link
Collaborator

@wolfgang-ch wolfgang-ch left a comment

Choose a reason for hiding this comment

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

Because of some text keys which I would have named the same way as other text keys, I've documented the structure of text keys which are currently applied https://mytourbook.sourceforge.io/mytourbook/index.php/development/dev-code-style/i18-code-style.

I also did some layout/code adjustments in the merged main branch that I don't have to review this again, except the review issues which are not system relevant.

@wolfgang-ch
Copy link
Collaborator

I also did some layout/code adjustments in the merged main branch

Before

mt-tooltip-before

After

mt-tooltip-after

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 17, 2021

Because of some text keys which I would have named the same way as other text keys, I've documented the structure of text keys which are currently applied https://mytourbook.sourceforge.io/mytourbook/index.php/development/dev-code-style/i18-code-style.

Ok, I will keep that documentation handy.
What does i18 mean ?

"Formatting In The Beginnings"
Is it ok when we modify old code that we update the formatting of those text keys ? or do you prefer keeping them as they are ?

I also did some layout/code adjustments in the merged main branch that I don't have to review this again, except the review issues which are not system relevant.

Sure. Sometimes it's better that you make the modifications so you can make the exact modifications you have in mind. That looks good to me.

@wolfgang-ch
Copy link
Collaborator

What does i18 mean ?

I removed the n from i18n (it looks better :-)

i18n: https://en.wikipedia.org/wiki/Internationalization_and_localization

@FJBDev
Copy link
Collaborator Author

FJBDev commented Feb 17, 2021

What does i18 mean ?

I removed the n from i18n (it looks better :-)

i18n: https://en.wikipedia.org/wiki/Internationalization_and_localization

Ok, I didn't know that.

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.

None yet

2 participants