openivity / openivity.github.io

An open-source fitness analytic platform offering data visualization (with OpenStreetMap), edit, convert, and combine multiple FIT, GPX, and TCX activity files. 100% client-side power! (WebAssembly)
https://openivity.github.io/
GNU General Public License v3.0
8 stars 1 forks source link

Change request regarding the HR Zone styling #54

Closed muktihari closed 9 months ago

muktihari commented 9 months ago

Hi Radit, I have styling requests regarding HR Zone component, before proceed please set device on browser to iphone 12/13 mini as a reference since it's arguably the smallest mobile phone right now.

  1. The icon is falling, it would be great if it's inline with the title, if the space is not available, consider suggestion on point (3)
  2. The text is a bit truncated
  3. If this question mark is taking space, what if we make the text always visible below the title? Since not all user know how to calculate HR zone, text suggestion "Common formula: Max HR = 220 - Age"
  4. Add more padding, reference is on figure 2
  5. Make this text a little more bottom, align with the Zone title

Additionally, change the color of the bar to be more vibrant like the other area graphs

IMG_2290 (figure 1)

IMG_2291 (figure 2)

raditzlawliet commented 9 months ago

case 1: TBD, if styling i prefer to remove the icon in title graph instead. Usually UI goes good if icon is in the left side, and theres already arrow to collapse. (will change other modules also)

case 2: OK, more width isn't bad, but i think some people still have that issue IF they use big font in mobile. Lets see later after increased.

case 3: OK, BUT, the req text is good, for the tooltip, im thinking to make it same with the Tools instead (same style less confuse keep simple)

case 4: OK, BUT, theres a less padding in the sample you give on the right side. (better to use same font size). more padding is good, lets make it same with right side sample. (so top/bot will same)

case 5: TBD what browser do you use? cause styling in my windows chrome already in the bottom. i prefer align bottom instead middle (cause it's small or subtitle).

so for now 2/3/4 can be executed

muktihari commented 9 months ago
  1. The icons are still required for our current layout-design so that our design doesn't look too plain. Unlike Strava, for example, have few icons in previous pages so they can go plain on graphs-page to make it "doesn't look too much" (or crowded). We can discuss this again when we do re-layouting after all features are complete. We might make some changes on the components' UI Element later based on the new layout design. Actually we can remove the icon for Heart Rate Zone for now (?)

  2. Let's take sample on majority people, they use normal font, and most mobile phone right now is more than 5" with more balance ratio, so they have more space as well. My phone on the other hand is the smallest, so I think it should be ok to add a bit more pixel (2-5px maybe).

  3. The idea behind question icon in Tools is because the help text is way too long so I added the option for user to collapse it when they think it no longer necessary. The "Fields Remover" for example doesn't have question mark since the help text is concise. For the formula it can be make concise so I think it doesn't really need it.

  4. Yes, please match with the right-side value (7:53/km) padding-bottom gap, actually it doesn't have to be exact same pixel, the most important thing is when we look at it, the weight of the visual-composition still look balance.

  5. Yes, what I mean is align-bottom, what I see right now is align-top. Actually similar to the pace graph "Average Pace" it should have been align-bottom with the right-side value, I will change it as well later. I tested it using safari and chrome for iOS, it looks the same (align-top) with chrome and firefox for macOS.

raditzlawliet commented 9 months ago

No need to make it complex, we do agree 2/3/4.

Actually for 1, I already mention commonly icon is used in the Start of Text, so that's why I'm prefer to remove it or move to left. For example Let's do only remove icon in the HR Zone (for example)

raditzlawliet commented 9 months ago
  1. Yes, what I mean is align-bottom, what I see right now is align-top. Actually similar to the pace graph "Average Pace" it should have been align-bottom with the right-side value, I will change it as well later. I tested it using safari and chrome for iOS, it looks the same (align-top) with chrome and firefox for macOS.

It seems chrome & edge via desktop already aligned bottom image image

I already check and it seems the problem is on Mobile Mode Only (edited: added some image) image

raditzlawliet commented 9 months ago

I already check and it seems the problem is on Mobile Mode Only

Updated, the problem aligned top when using Touch Mode

raditzlawliet commented 9 months ago

https://github.com/openivity/openivity.github.io/blob/0cb2dd810b5abbbce8907a0d537185833c8e38a1/src/assets/main.scss#L111 I've see the Case 5 (aligned top problem) is from here,

But before adjusted, @muktihari is there a reason why it using scope pointer for this style? Is it intended smaller screen or touch screen?

muktihari commented 9 months ago

Ah alright, I intended make base of font-size for all touch-screen-only-devices to be smaller than desktop, it will not affect hybrid-device such as laptop that has touch-screen, only mobile devices. What can we do about it if that's the case?

raditzlawliet commented 9 months ago

I intended make base of font-size for all touch-screen-only-devices to be smaller than desktop,

If it only intended for mobile, why we don't use screen (responsive)? so for mobile/tablet with large screen will treat as large screen (similar with desktop)

raditzlawliet commented 9 months ago

If it intended for mobile small screen, i'll change it into spesific for "smaller-size" screen (< sm or md in bootstrap)

raditzlawliet commented 9 months ago

If it intended for mobile small screen, i'll change it into spesific for "smaller-size" screen (< sm or md in bootstrap)

only for this case (HR Zone), for other issue #58 , let's do it later after discuss more properly

muktihari commented 9 months ago

If it only intended for mobile, why we don't use screen (responsive)? so for mobile/tablet with large screen will treat as large screen (similar with desktop)

The previous original layouting of bootstrap does not fit my iphone mini, we need to consider does sm or md of the original boostrap to fit properly on smaller devices when we do re-layouting later. And with my limited css knowledge limited time to handle that, that's what I could do.

If the scoped font-size is affecting the alignment, I think let be it for now, we will include it on re-layouting time.

raditzlawliet commented 9 months ago

Ok, that's pretty clear, I'll try Case 5 spesific for HRZone if it can. otherwise let ignore it till relayout.

raditzlawliet commented 9 months ago

Also, You mention about reduce opacity in progress-bar background (that didn't fill) in Split Pace.

This is good, if HR Zone progress bar also need focus, we can change the opacity to 0.6-0.8 together

muktihari commented 9 months ago

I think this progress bar is good we can keep it as it is, I only have concern with the Split Pace's progress bar

raditzlawliet commented 9 months ago

Ok, pretty much concluded.