Do not scroll list label if <scroll>false</scroll> #4247

Merged
merged 1 commit into from Dec 22, 2014

Projects

None yet

6 participants

@Black09
Member
Black09 commented Feb 20, 2014

With the recent change of commit e4753e8 it wasn't possible anymore to disable scrolling with <scroll>false</scroll>. This fixes it.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Feb 21, 2014
@jmarshallnz jmarshallnz commented on an outdated diff Apr 26, 2014
xbmc/guilib/GUIControlFactory.cpp
@@ -996,7 +1015,7 @@ CGUIControl* CGUIControlFactory::Create(int parentID, const CRect &rect, TiXmlEl
XMLUtils::GetFloat(pControlNode, "itemgap", buttonGap);
XMLUtils::GetInt(pControlNode, "movement", iMovementRange);
GetAspectRatio(pControlNode, "aspectratio", aspect);
- XMLUtils::GetBoolean(pControlNode, "scroll", bScrollLabel);
+ GetScrollValue(pControlNode, "scroll", scrollValue);
@jmarshallnz
jmarshallnz Apr 26, 2014 Member

I think you can do this via

bool temp;
if (XMLUtils::GetBoolean(pControlNode, "scroll", temp))
  scrollValue = temp ? CGUIControl::ALWAYS : CGUIControl::NEVER;
@jmarshallnz jmarshallnz commented on an outdated diff Apr 26, 2014
xbmc/guilib/GUIListLabel.cpp
@@ -43,7 +43,14 @@ CGUIListLabel::~CGUIListLabel(void)
void CGUIListLabel::SetScrolling(bool scrolling)
{
- m_label.SetScrolling(scrolling || m_alwaysScroll);
+ if (m_scroll == CGUIControl::FOCUS)
+ {
+ m_label.SetScrolling(scrolling);
+ }
+ else
+ {
+ m_label.SetScrolling((m_scroll == CGUIControl::ALWAYS) ? true : false);
+ }
@jmarshallnz
jmarshallnz Apr 26, 2014 Member

the braces aren't really needed.

@mkortstiege
Member

@Black09 mind updating according to the comment so we can get this in?

@Black09
Member
Black09 commented Jul 12, 2014

Updated.

@jmarshallnz
Member

Seems you still need rebasing?

@Black09
Member
Black09 commented Jul 20, 2014

Did the rebase.

@MartijnKaijser
Member

jenkins build this please

@MartijnKaijser
Member

@topfs2 up for consideration?

@HitcherUK
Contributor

@ronie @MartijnKaijser @mkortstiege
Is this still on for Helix?
Also, does it add the ellipsis so focused and non focused labels look the same in a list control?
Thanks.

@ronie
Member
ronie commented Dec 22, 2014

i'm afraid this one went under the radar and won't make it into Helix.
(it should have since it fixes a regression)

i will test this patch later today and if no issues found,
I'll give it my best shot to get this in our 14.1 release.

@ronie
Member
ronie commented Dec 22, 2014

tested and works fine.
@HitcherUK yes it shows the ellipsis on both focused and unfocused items.

@ronie ronie merged commit 4aa343d into xbmc:master Dec 22, 2014

1 check passed

default Merged build finished.
Details
@ronie
Member
ronie commented Dec 22, 2014

backport for 14.1:
#6011

@HitcherUK
Contributor

@ronie I've just noticed that the default (scroll=true) for focused items doesn't work anymore. Isn't the intended behaviour meant to be true for focused items and false for non focused items if they're not defined by the skin?

Thanks.

@ronie
Member
ronie commented Jan 16, 2015

hope you don't mind if i pass that question to @Black09

@ronie
Member
ronie commented Jan 16, 2015

seems to work as expected in Confluence though.
i've tested List view (CommonRootView), label scrolls when focused, doesn't scroll when unfocused.

@HitcherUK
Contributor

@ronie @Black09 Sorry guys, for some reason I had false for labels in my defaults.xml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment