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

[guilib] add justify alignment for grouplist controls #7809

Merged
merged 1 commit into from Oct 3, 2015

Conversation

mkortstiege
Copy link
Member

Adds justified alignment for grouplist controls. This is done by re-calculating the item gap size based on the actual control width and the amount of items. Needs testing.

/cc @BigNoid, @HitcherUK, @phil65 and @ronie

@mkortstiege mkortstiege added the Type: Improvement non-breaking change which improves existing functionality label Aug 14, 2015
itemsCount++;
}
}
m_itemGap = (Size() - itemsSize) / itemsCount;

This comment was marked as spam.

This comment was marked as spam.

@BigNoid
Copy link
Member

BigNoid commented Aug 14, 2015

Works as expected, thx

@mkortstiege
Copy link
Member Author

Added the itemsCount check to avoid possible division by zero and rebased.

@phil65
Copy link
Contributor

phil65 commented Aug 14, 2015

I am not sure if the behaviour is ideal in case the width of all items combined is higher than the width of the grouplist. At the moment it seems to assign negative itemgap values (which leads to overlapping buttons). Perhaps scrolling would be nicer in that case?

@BigNoid
Copy link
Member

BigNoid commented Aug 14, 2015

There should be a check probably that it only uses the calculated item gap if its higher than the item gap set in the grouplist?

@mkortstiege
Copy link
Member Author

There should be a check probably that it only uses the calculated item gap if its higher than the item gap set in the grouplist?

Jep, that's what i had in mind as well. Should it also modify the group width and start scrolling like @phil65 suggested?

@BigNoid
Copy link
Member

BigNoid commented Aug 14, 2015

Yes that would be best I think to prevent cut off items.

@phil65
Copy link
Contributor

phil65 commented Aug 14, 2015

You want to adjust the width of the grouplist itself? I dont think it´s a good idea, better have cut-off items than to not respect the values the skinner has set. Ideally (but that´s perhaps out of scope) we would have something like <itemgap min="xx" max="xx>xx</itemgap> I think?

@BigNoid
Copy link
Member

BigNoid commented Aug 14, 2015

I assumed the width would only get smaller because its supposed to scroll. It shouldn't exceed the width set in the grouplist width tag I agree.

@BigNoid
Copy link
Member

BigNoid commented Aug 15, 2015

With last commit it doesnt overlap controls anymore. Note that this wont work as expected with images that have transparent borders. Eg, you will want to set the itemgap to a lesser value to compensate for the transparent borders, but the calculated itemgap is then higer and that will be picked. This is for me better than to have overlapping controls though.

@mkortstiege
Copy link
Member Author

Squashed and rebased. How do we proceed with this one now? /cc @BigNoid, @HitcherUK, @phil65, @ronie

@mkortstiege mkortstiege added the Type: Feature non-breaking change which adds functionality label Aug 19, 2015
@Hitcher
Copy link
Contributor

Hitcher commented Aug 19, 2015

Trying a test build later.

@Hitcher
Copy link
Contributor

Hitcher commented Aug 23, 2015

All good here. Thanks.

@mkortstiege
Copy link
Member Author

What's the status here. Is it working as intended and of any use?

@phil65 phil65 self-assigned this Sep 12, 2015
@@ -208,6 +208,8 @@ bool CGUIControlGroupList::OnMessage(CGUIMessage& message)

void CGUIControlGroupList::ValidateOffset()
{
// calculate item gap

This comment was marked as spam.

@mkortstiege mkortstiege force-pushed the grouplist-justify branch 2 times, most recently from f34f6ee to cdf4471 Compare October 2, 2015 07:34
@mkortstiege
Copy link
Member Author

Rebased. Do we want justified alignment for grouplists or should it be closed? jenkins build this please.

@phil65
Copy link
Contributor

phil65 commented Oct 2, 2015

I´d say yes, shove it in.

@Hitcher
Copy link
Contributor

Hitcher commented Oct 2, 2015

It's working as intended so I see no reason not to include it.

@mkortstiege mkortstiege added this to the Jarvis 16.0-alpha4 milestone Oct 3, 2015
mkortstiege added a commit that referenced this pull request Oct 3, 2015
[guilib] add justify alignment for grouplist controls
@mkortstiege mkortstiege merged commit efca3e1 into xbmc:master Oct 3, 2015
@mkortstiege mkortstiege deleted the grouplist-justify branch October 6, 2015 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality Type: Improvement non-breaking change which improves existing functionality v16 Jarvis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants