Storm-844 "More" should be "Less" when Media Control is open


Submitter:	Jonathan Yap	Reviewers
Branch:	2.5	Groups:	viewer
Bugs:	storm-844	People:
Change Number:	None	Repository:	viewer-development

Description:

"More" should be "Less" when Media Control is open

This is a trivial text change in an xml file. The reason I am posting this here is due to some confusion using label_selected. In this case having it set to a different value than when label is set to seems to have no effect, so I have made them identical.

I scanned all the xml files and there are only about 5 places where label_selected is different from the preceding label= value.

Is there any reason to revert back to having them set to different values?
i.e. label="More" and label_selected="Less"

Testing Done:

Expand All

doc/contributions.txt (Diff revision 1)

Sorting issue numbers in doc/contributions.txt is a Good Thing(TM), but unrelated to the issue at hand, thus should happen in a separate commit.

Oz Linden Jan. 12, 2011, 6:45 a.m. (Jan. 12, 2011, 6:45 a.m.)

Actually, because I use changes to this file to generate some of our metrics for open source contributions, I'd prefer that any existing order be preserved: that is, add lines as needed (and adding them in order is nice), but do not reorder existing lines please.

Tested your patch and after testing it works however you do not need to have label_selected at all it works as expected without to

To me, this is the wrong solution.  label_selected used to work to allow a button to display different text when it was selected, so you could have a button that said More until it was pressed or selected and displayed more informaiton.  At that time the text could change to Less indicating that pressing the button a second time would maybe reduce the information displayed.  If the button in question here doesn't change when pressing, that means that something down in the control code is wrong.

Jonathan Yap Jan. 13, 2011, 8:56 a.m. (Jan. 13, 2011, 8:56 a.m.)

I did a test with the floater preview UI (one of the few places that label is not the same as label_selected.  There is a >> button there that does change to << when pressed.

However, in this case, there is some code that does the button swapping.  In llpanelnearbymedia.cpp / void LLPanelNearByMedia::onMoreLess()

	getChild<LLUICtrl>("more_btn")->setVisible(!is_more);
	getChild<LLUICtrl>("less_btn")->setVisible(is_more);

If people think refactoring rather then just rewording is the way to go please say so.  I always hesitate to "fix what ain't broke".

The vast majority of button definitions in the xml files have identical entries for label and label_selected (is having label_selected present a requirement in a button definition or are all these entires superfluous?).

Twisted Laws Jan. 14, 2011, 6:46 a.m. (Jan. 14, 2011, 6:46 a.m.)

I looked into this and have a patch that corrects it.  What the patch does in llPanelNearByMedia::onMoreLess(), the first statement is changed to bool is_more = getChild<LLButton>("more_btn")->getToggleState();  the two statements Jonathan shows are changed to just one getChild<LLUICtrl>("more_btn")->setVisible(true); (may just be removed and not be necessary) and in the xml file, the first more_btn gets is_toggle="true" added and the less_btn is removed.  My patch file is attached to STORM-844.

Jonathan Yap Jan. 16, 2011, 10:03 a.m. (Jan. 16, 2011, 10:03 a.m.)

I applied this patch and you can view it via the link to the repository in the jira.

Seems correct. Ship it modulo the "don't shuffle the contributions" mentioned by Oz.

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.