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

Change request regarding the HR Zone styling #54

Closed
muktihari opened this issue Dec 1, 2023 · 15 comments
Closed

Change request regarding the HR Zone styling #54

muktihari opened this issue Dec 1, 2023 · 15 comments
Assignees
Labels
UI Changes related to UI Element

Comments

@muktihari
Copy link
Member

muktihari commented Dec 1, 2023

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)

@muktihari muktihari added the UI Changes related to UI Element label Dec 1, 2023
@raditzlawliet
Copy link
Member

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
Copy link
Member Author

muktihari commented Dec 1, 2023

  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
Copy link
Member

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
Copy link
Member

raditzlawliet commented Dec 2, 2023

  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
Copy link
Member

raditzlawliet commented Dec 2, 2023

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

Updated, the problem aligned top when using Touch Mode

@raditzlawliet
Copy link
Member

font-size: 13px;

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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

@raditzlawliet
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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

@raditzlawliet
Copy link
Member

raditzlawliet commented Dec 2, 2023

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
Copy link
Member Author

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
Copy link
Member

Ok, pretty much concluded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Changes related to UI Element
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants