vsergeev / briefsky

A free weather frontend to a variety of weather providers
https://briefsky.app/
Other
244 stars 24 forks source link

Create vertical layout option #11

Open mhkeller opened 1 year ago

mhkeller commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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
vsergeev commented 1 year ago

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

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.

mhkeller commented 1 year ago

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.

vsergeev commented 1 year ago

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

mhkeller commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

mhkeller commented 1 year ago

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 commented 1 year ago

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.

vsergeev commented 1 year ago

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

mhkeller commented 1 year ago

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

vsergeev commented 1 year ago

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 commented 1 year ago

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

msft-jeelpatel commented 8 months ago

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?

mhkeller commented 8 months ago

@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 commented 7 months ago

Reopening if other ppl want to work on it

patel-jeel92 commented 7 months ago

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

vsergeev commented 7 months ago

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

patel-jeel92 commented 7 months ago

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!