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

No more % based font-sizes!!! #471

Merged
merged 12 commits into from
Mar 13, 2015
Merged

No more % based font-sizes!!! #471

merged 12 commits into from
Mar 13, 2015

Conversation

jasoncalabrese
Copy link
Member

Also some more html/css refactoring and clean up, needs tons of testing on lots of devices and browers

@francescaneri
Copy link
Contributor

Hi Jason, test iphone 4S, IPAD 2, SAMSUNG NEXUS (OK)

@jasoncalabrese
Copy link
Member Author

Thanks for your help testing @francescaneri, I'm planning to take a 2nd pass at this tonight and see if I can simplify the media queries in main.css

@jimsiff
Copy link
Contributor

jimsiff commented Mar 10, 2015

Some of us on support chat have tested on iphone 6+, iPad Mini, iPad Mini 2, Moto G, Galaxy S3, and Kindle Fire. I will test a 5s and 6 tonight. I have tested in Safari and Chrome on a Yosemite Macbook. I will test a Windows PC later.

I like what you have so far. It works pretty well as-is on most platforms I've tried, but I did find some minor issues. The larger x/y axis fonts looks good on mobile browsers. It's subtle but makes it easier to see. The lighter bdbdbd font folor noticeably improves contrast for b/w text.

Issues

  • Time and BG are side by side in portrait mode on 6+, so trend is clipped.

img_1288

  • X axis font is clipped in landscape mode on 6+. This exists in master... not sure why I didn't catch this before now.

img_1285

  • Desktop browser x/y axis font size break points might need some adjustment. Just about where the time and BG fonts switch to the smaller size, the x/y font size is noticeably larger than either a smaller or larger window.

X/Y fonts too big... Time and BG fonts small

screen shot 2015-03-10 at 11 37 33 am

X/Y fonts ok... Time and BG fonts large
screen shot 2015-03-10 at 11 37 43 am

X/Y fonts ok... Time and BG fonts small
screen shot 2015-03-10 at 11 37 51 am

  • Site renders poorly on the S3 using the default browser. Chrome is fine. SGV dots are rendered blue, long term trend line dots do not look right. I think this might be a known issue with the Samsung browser.

Suggestions

  • Time / Time ago font size could be slightly larger. Using a desktop browser, the font size is smaller in wip/no-percents vs. current master.
  • Could time and BG font size be the same? They both are a larger font with one or more status pills below them.
  • Could time ago and currentDetails font size be the same? Same reasons as above.
  • Could you change time, time ago color to bdbdbd so all b/w text is consistent? Not sure how it would affect night mode.
  • Could you normalize pill size, shape of time ago to newer currentDetails style?

@francescaneri
Copy link
Contributor

I noticed that using the application will not experience the problems that reported jimsiff
I hope you find it useful Matteo

@francescaneri
Copy link
Contributor

image

@francescaneri
Copy link
Contributor

image

@francescaneri
Copy link
Contributor

image

@francescaneri
Copy link
Contributor

image

@francescaneri
Copy link
Contributor

I put in comparing the two versions % - no %
I hope that there is help

@jasoncalabrese
Copy link
Member Author

@jimsiff I'm thinking #bdbdbd should be used for the BG in non-color mode, but maybe not for time/time ago.

I left the time/timeago the darker grey and made time smaller than the BG so that the most important info is more glanceable. Aligning the timeago style with the new pills makes sense.

I was hoping to work on this more last night, but didn't get to it.

@jasoncalabrese
Copy link
Member Author

@francescaneri has reported some issues with the snooze menu not working on an Phone 5, iPhone 3GS. Can anyone confirm that issue? I have an idea for a fix, but would be nice to confirm the bug first.

@francescaneri
Copy link
Contributor

img_0013 1
iphone 3gs horizontal screen, the setting pannel it works
while no vertical screen
I meat settings panel and not snooze menu. sorry

@francescaneri
Copy link
Contributor

the snooze menu not working with chrome - (firefox and safari and explorer 11 ok)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.52% when pulling 89129e4 on wip/no-percents into 1e27704 on dev.

@jasoncalabrese
Copy link
Member Author

@jimsiff I tried using #bdbdbd for time/timeago and matching the font size, it felt distracting to me. At the same time the sizes and color not being symmetrical doesn't feel right. Many of the other issues should be addressed, going to put this on my prod site before going to bed.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.52% when pulling 912bf3a on wip/no-percents into 1e27704 on dev.

@francescaneri
Copy link
Contributor

image
iPad 2

@francescaneri
Copy link
Contributor

image

iPhone 4S horizontal
that there will be never used . lock the screen vertically both smartphone and iPad
You simplify things

@francescaneri
Copy link
Contributor

image
iPhone 5

@francescaneri
Copy link
Contributor

image
iPhone 4S

@jimsiff
Copy link
Contributor

jimsiff commented Mar 11, 2015

Jason, looks good.

Your fix for portrait mode worked... looks good on the 6+ now.

img_1297

I noticed two minor issues...

With the larger font size, the x and y axis can get crowded where they meet at the lower right edge of the graph... even with a mid-size browser window. The issue is especially noticeable when a new hour is displayed at the right edge of the x axis. The issue gets more pronounced as the window is shrunk vertically.

New font size

screen shot 2015-03-11 at 12 32 02 pm

Old font size

screen shot 2015-03-11 at 12 31 43 pm

The x axis fonts are clipped on the iPhone 6+ when in landscape mode.

img_1298

@jasoncalabrese
Copy link
Member Author

@jimsiff, Think I fixed the font issues, but wasn't able to reproduce the iPhone 6+ X croping

@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.52% when pulling 5e970c4 on wip/no-percents into 1e27704 on dev.

@francescaneri
Copy link
Contributor

immagine
you can not turn down a few points on the y axis so as to have no problem with iphone 3gs and 4s and 5 ?
img_1686

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.48% when pulling 631ae67 on wip/no-percents into 1e27704 on dev.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.48% when pulling 758b868 on wip/no-percents into 1e27704 on dev.

@jasoncalabrese
Copy link
Member Author

when there is future data:

in the future

time ago warning and urgent levels

warning stale data

urgent stale data

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.48% when pulling 967f519 on wip/no-percents into 1e27704 on dev.

@jimsiff
Copy link
Contributor

jimsiff commented Mar 12, 2015

👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.48% when pulling 3a22002 on wip/no-percents into 1e27704 on dev.

@jasoncalabrese
Copy link
Member Author

@francescaneri figured out how to move the x-axis labels to the bottom, also now using the full height of the chart. 20 more pixels!!!

use the full height

@francescaneri
Copy link
Contributor

very well!! ;)

@scottleibrand
Copy link
Member

Should we also expand the Y-axis so that 400 is the maximum instead of
500? Since we moved the treatments in-line, I don't think we need the
buffer at the top any longer... And since you moved the x-axis labels
below, we might be able to do the same at the bottom...

On Thu, Mar 12, 2015 at 10:03 AM, Jason Calabrese notifications@github.com
wrote:

@francescaneri https://github.com/francescaneri figured out how to move
the x-axis labels to the bottom, also now using the full height of the
chart. 20 more pixels!!!

[image: use the full height]
https://cloud.githubusercontent.com/assets/751143/6623242/f6f9b1f6-c89e-11e4-93ea-804be6b3acae.png


Reply to this email directly or view it on GitHub
#471 (comment)
.

@francescaneri
Copy link
Contributor

@scottleibrand therefore also HIGH ;) ...and if we can integrate IOB - COB with the new style

@jimsiff
Copy link
Contributor

jimsiff commented Mar 12, 2015

Looks like the straight down (and up?) button arrows might be a touch too big.

img_1306

@jasoncalabrese
Copy link
Member Author

@scottleibrand I was thinking about a dynamic scale based on the range within the time period. I'm thinking basal and combo/extended will still go at the top so we need some padding.

@scottleibrand
Copy link
Member

That would be good. We should probably include the user-configured high
and low thresholds as the default min and max for the range, and expand it
if the displayed data goes outside that, of course.

-Scott

On Thu, Mar 12, 2015 at 5:43 PM, Jason Calabrese notifications@github.com
wrote:

@scottleibrand https://github.com/scottleibrand I was thinking about a
dynamic scale based on the range within the time period. I'm thinking basal
and combo/extended will still go at the top so we need some padding.


Reply to this email directly or view it on GitHub
#471 (comment)
.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 72.48% when pulling 50dd57d on wip/no-percents into 1e27704 on dev.

@jasoncalabrese
Copy link
Member Author

That's exactly what I was thinking @scottleibrand, that should be a new PR will take some expermenting

@jimsiff I made the arrow a little smaller with the max-width: 750px breakpoint, I'm going to merge this to dev now. We can continue to polish as we get everything thing else ready for enchilada.

jasoncalabrese added a commit that referenced this pull request Mar 13, 2015
@jasoncalabrese jasoncalabrese merged commit 2639e57 into dev Mar 13, 2015
@jasoncalabrese jasoncalabrese deleted the wip/no-percents branch March 13, 2015 00:52
@francescaneri
Copy link
Contributor

image
@jasoncalabrese font size check time thanks

@jasoncalabrese jasoncalabrese modified the milestone: enchilada Mar 18, 2015
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.

5 participants