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

replace header background image #3971

Merged
merged 1 commit into from Jan 7, 2014
Merged

Conversation

ronie
Copy link
Member

@ronie ronie commented Jan 5, 2014

this is a cleanup of #3940

it replaces the header background texture in a number of places, to improve readability.

for @universal

readability

@jmarshallnz
Copy link
Contributor

Nice!

@MartijnKaijser
Copy link
Member

Better indeed

@un1versal
Copy link
Contributor

Yes indeed, That's what I was working towards, 👍

About the header label font types? All dialogs use font13_title except settings/info/profile. Because however font13_title is shorter doesn't look as good throughout.

Would you like me to have a stab at these and make them all font16? And if so the dialogheader.png now also needs to stretch to 52 because this texture is set at 40 in these other places too.. there's quite a few more of these though ;-)

In any case this is 20 steps in right direction, thank you @ronie.

@un1versal
Copy link
Contributor

@ronie

To my comments above to this PR I pulled your changes into a branch and added my changes ontop.
Neither a PR in any case its so you can see and review. without saturating emails.

  • dialogheader.png height to 52 as per ronie PR
  • Header Label font unification atm thers font13_title (which looks bleh in some dialogs) and font16 which looks great in all
  • Them applying both of those ideas throughout skin.

un1versal@68dccb4 refines the header label fonts in weather and fixes alignment (somehow 20 was a bit too low)

un1versal@6744513 Really takes this a step further and looks at what I could find, that was sort of applicable to the points above.
One note is that perhaps some dialog types dont need dialogheader height to 52 though would benefit from font16.
This one needs testing and checking,

  • PVR dialogs changes because I cant test PVR dialogs.
  • Peripherals Dialogs because I dont have any that trigger those either.

I dondt wanna make a commnet without showing what I was looking at.

@da-anda
Copy link
Member

da-anda commented Jan 6, 2014

In general +1, but not sure I like it in it's current state. Settings header is visually better, but the font should be aligned left as it's no window header (not spanning the entire window), plus it would "jump" less when you switch settings sections. But that's probably just my taste.
And at least form the screenshots the weather headers don't look like headers anymore - the visual distinction is a bit too subtile IMO. Readability is ofc ways better. Maybe give the dividing bottom line more opacity and make it reach further to the left and right edges (probably stretch the subtle background color to same width)?

@ronie
Copy link
Member Author

ronie commented Jan 7, 2014

@da-anda the label is centered because the highlight is also centered. it looks better that way imo.
there's no jumping involved btw, when you switch settings categories, the panel will zoom out/in.

i've updated the image as you suggested:

readability-update

Would you like me to have a stab at these and make them all font16?

@UniversaI the fonts on the dialogs should remain as they are.
the dialog header label can be very long and with a bigger font it might not fit anymore.

And if so the dialogheader.png now also needs to stretch to 52 because this texture is set at 40 in these other places too

nope, you can't compare the size of an image on a fullscreen window to the same image in a small dialog.
they should be in proportion.

@zag2me
Copy link
Contributor

zag2me commented Jan 7, 2014

Just to add from a design point of view, it looks good centered to me.

@da-anda
Copy link
Member

da-anda commented Jan 7, 2014

good for me now, thanks @ronie (the only thing I don't like now is the way too big weather provider logo overlaping everything, but that's a different story)

@ronie
Copy link
Member Author

ronie commented Jan 7, 2014

tweaked the weather stuff as suggested (smaller logo, better alignment of header label)

ronie added a commit that referenced this pull request Jan 7, 2014
@ronie ronie merged commit 683a7b5 into xbmc:master Jan 7, 2014
@ronie ronie deleted the confluence-readability branch February 26, 2016 23:40
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.

None yet

6 participants