briefsky icon indicating copy to clipboard operation
briefsky copied to clipboard

Create vertical layout option

Open mhkeller opened this issue 2 years ago • 20 comments
trafficstars

https://github.com/vsergeev/briefsky/issues/9 Creates an option in settings to have a vertical layout, similar to Dark Sky

Here's a first go at it. Needs more mobile testing and check if the math is right aligning the times alongside the vertical bar and that the temperatures are scaled correctly.

Let me know what you think.

Screen Shot 2023-04-08 at 1 15 10 AM

mhkeller avatar Apr 08 '23 05:04 mhkeller

I'm playing around with some details for aligning the weather timestamps to the vertical bar. In Dark Sky, it appears that the timestamp falls near the middle of that bar. So hours 8am and 10am are labeled and rain starts at 9am the blue bar will start halfway between 8am and 10am. Setting a height of 100 / 12 sets the label a little differently. I'm trying to get it to align center without throwing off the first label... Work in progress...

mhkeller avatar Apr 08 '23 16:04 mhkeller

@vsergeev I think it's ready for you to take a look. Let me know what you think: Screen Shot 2023-04-08 at 4 35 34 PM

mhkeller avatar Apr 08 '23 20:04 mhkeller

@mhkeller thanks, this looks like a good start. I have a couple of feedback items:

  • I think it would look better at 80% width and centered
  • The temperature pills aren't centered with the line for me
  • I'd like to avoid any fixed pixel heights on containers (e.g. height: 525px for the container in HourlyDetails)
  • Commit 9679c95 appears to remove text colors and drop shadows, which breaks the formatting on the horizontal view. (Why were these deleted?)
  • No need to commit the production bundle -- I can do that on an actual release with a version number change

The commits can also be squashed down into two commits, one that adds the layout setting (first commit), and one that adds the vertical layout.

vsergeev avatar Apr 14 '23 08:04 vsergeev

Woops on 9679c95. I'll revert that and work on the other items. For the height, what would you prefer, something like 80vh / 90vh / 100vh?

I noticed that there is no sunrise / sunset information for the current day anymore because that is only in the expandable component. I'll add those elements to the text information at the top.

mhkeller avatar Apr 14 '23 13:04 mhkeller

Yeah, I think a relative view height would be good in place of the fixed height.

vsergeev avatar Apr 14 '23 15:04 vsergeev

Some updates:

The temperature pills aren't centered with the line for me

This should be fixed.

I'd like to avoid any fixed pixel heights on containers (e.g. height: 525px for the container in HourlyDetails)

Fixed. Removing the height seemed to work fine.

Commit 9679c95 appears to remove text colors and drop shadows, which breaks the formatting on the horizontal view. (Why were these deleted?)

Fixed

No need to commit the production bundle -- I can do that on an actual release with a version number change

Fixed

The commits can also be squashed down into two commits, one that adds the layout setting (first commit), and one that adds the vertical layout.

Fixed

I think it would look better at 80% width and centered

This is the only thing that didn't work out well.The width becomes too narrow for there to be much variation on the temperature scale. Here's a screenshot. IMG_2964

Separately:

I added the sunrise and sunset times back and I standardized their look between the two components.

mhkeller avatar Apr 15 '23 04:04 mhkeller

I increased the padding in some places for better tap accessibility and lightened up the temperature range so it's more easily scannable to the eye.

mhkeller avatar Apr 15 '23 20:04 mhkeller

One other thing that's missing from today's view is when it's raining, it should display the Precipitation and Amount fields.

mhkeller avatar Apr 15 '23 20:04 mhkeller

Sure thing. I incorporated some accessibility and duplicate content fixes that I encountered but can create new issues for those. I can explain the reasoning for them in those issues and would be in favor of including them since there is some duplicate information that would adversely affect the UX for the vertical layout.

mhkeller avatar Apr 19 '23 05:04 mhkeller

The temperature pills are vertically aligned with the lines, but I think it would look better if the hour and description on the left were vertically aligned with the lines too.

Can you explain what you mean by this? Do you mean the horizontal line looks like it's not at the centerline of the text? Maybe you can send a screenshot of what you're seeing so I can make sure the fix is getting at the right thing.

mhkeller avatar Apr 19 '23 05:04 mhkeller

The temperature pills are vertically aligned with the lines, but I think it would look better if the hour and description on the left were vertically aligned with the lines too.

Can you explain what you mean by this? Do you mean the horizontal line looks like it's not at the centerline of the text? Maybe you can send a screenshot of what you're seeing so I can make sure the fix is getting at the right thing.

I think so. Sure, here is a screenshot:

alignment

vsergeev avatar Apr 19 '23 05:04 vsergeev

Got it. Yea that looks different from mine. What browser and OS are you using so I can try to replicate?

mhkeller avatar Apr 19 '23 05:04 mhkeller

Got it. Yea that looks different from mine. What browser and OS are you using so I can try to replicate?

I'm using Firefox 111 on Linux. I see it in Google Chrome 111 too.

vsergeev avatar Apr 19 '23 05:04 vsergeev

I've added a View tab to the Settings on master, which will be a better place for a layout option like this.

vsergeev avatar Apr 27 '23 04:04 vsergeev

The temperature pills are vertically aligned with the lines, but I think it would look better if the hour and description on the left were vertically aligned with the lines too.

Can you explain what you mean by this? Do you mean the horizontal line looks like it's not at the centerline of the text? Maybe you can send a screenshot of what you're seeing so I can make sure the fix is getting at the right thing.

I think so. Sure, here is a screenshot:

alignment

Hi @mhkeller. Were you able to address this?

msft-jeelpatel avatar Jan 14 '24 21:01 msft-jeelpatel

@msft-jeelpatel, I didn't have the bandwidth to test this out since it was fine on my machine and I don't a Linux to test on. I won't have the time to finish this PR but if someone wants to pick it up, feel free to! I'll close this and if someone wants to resurrect it, then by all means they should

mhkeller avatar Jan 14 '24 22:01 mhkeller

Reopening if other ppl want to work on it

mhkeller avatar Jan 23 '24 16:01 mhkeller

Reopening if other ppl want to work on it

I wish I knew how to fix some of the layout issues :/.

For example, the "Now" starts at the very top of the weather bar image

Compared to DarkSky it looked like there was equal padding at the top and bottom. Currently, since the "Now" starts at the very top margin of the weather bar, there is quite a lot of unused space at the bottom

image

Other than that, I can try taking this forward

patel-jeel92 avatar Jan 23 '24 21:01 patel-jeel92

@patel-jeel92 I can help with some of the formatting issues to get this PR to the finish line.

vsergeev avatar Jan 25 '24 09:01 vsergeev

Hi @vsergeev I have started a new PR #21 to add the vertical layout. The reason i chose to add a new PR is because i have forked the repo and created a new branch off of it. Plus this had a bunch on conflicts and i found it easier to start over.

Huge thanks to @mhkeller for starting the initial work!

patel-jeel92 avatar Jan 25 '24 15:01 patel-jeel92