STORM-1095 Chat preferences > font size should increase size of input text in the chat box


Submitter:	Jonathan Yap	Reviewers
Branch:	2.6.2	Groups:	viewer
Bugs:	STORM-1095	People:
Change Number:	None	Repository:	viewer-development

Description:

 Chat preferences > font size should increase size of input text in the chat box

Testing Done:

Change font size in preferences and see
1) Font size in chat input box changes to new size immediately
2) Font size is set to selected size when viewer is restarted

Expand All

Diff
- added Diff r2

Description:

Cleanup of where a few .h files were included.

indra/llui/lllineeditor.h (Diff revision 2)

Any reason why ...

indra/llui/lllineeditor.cpp (Diff revision 2)

... this simple setter isn't implemented right at the declaration (i.e. in indra/llui/lllineeditor.h)?

Jonathan Yap April 4, 2011, 3:48 a.m. (April 4, 2011, 3:48 a.m.)

Unfortunately this identical change is also in Storm-1094 and I don't want to make the MM crazy.  As I do not have much internet at the moment if someone would like to adjust both of these jiras to match please do so, but I don't see that this is a showstopper if not addressed.

indra/newview/llbottomtray.cpp (Diff revision 2)

If font isn't used later, this could be written as
	mNearbyChatBar->getChatBox()->setFont(LLViewerChat::getChatFont());

(Not sure which variant is more readable.)

Jonathan Yap April 4, 2011, 3:50 a.m. (April 4, 2011, 3:50 a.m.)

I was doing a bit of code stealing of my changes in other files, hence the two lines made more sense in this instance.

indra/newview/llnearbychatbar.cpp (Diff revision 2)

Put some punctuation between "font" and "whohoo" :-P

indra/newview/llviewerchat.cpp (Diff revision 2)

Whatever the reason for that comment is, I guess the idea is that you put new members below it, not above.

Also, should this be initialized to null here ...

Jonathan Yap April 4, 2011, 3:52 a.m. (April 4, 2011, 3:52 a.m.)
```
This was fixed as well.
```

indra/newview/llviewerchat.cpp (Diff revision 2)

... so that it actually will be null here the first time?

Boroondas comments should be addressed, especially the one here under...

indra/newview/llviewerchat.cpp (Diff revision 2)

Should be initialized to NULL.

Jonathan Yap April 4, 2011, 3:46 a.m. (April 4, 2011, 3:46 a.m.)

I fixed that initialization to NULL issue and that change was in the last PO test build.

indra/newview/llnearbychatbar.h (Diff revision 2)

Avoid redundant changes.

indra/newview/llnearbychatbar.h (Diff revision 2)

Remove the class prefix: it may cause compilation problems on Linux.

Oz Linden April 5, 2011, 1:10 p.m. (April 5, 2011, 1:10 p.m.)

Does cause failure on Linux... I fixed it to do the PO Review build and forgot to mention it.  Good catch, Vadim.

indra/newview/llnearbychatbar.cpp (Diff revision 2)

CS: "if ("

indra/newview/llviewerchat.h (Diff revision 2)

* Readability: please separate type definitions from methods.
* CS: Inconsistent naming. Should be something like font_change_signal_t.

indra/newview/llviewerchat.h (Diff revision 2)

* Memory leak? Seems to be never deleted.
* Why pointer?
* Move to private section.
* CS: rename to sFontChangedSignal.

Diff
- added Diff r3

Looks good.

Some minor stuff still:

indra/newview/llviewerchat.h (Diff revisions 2 - 3)

Why repeat the 'public:' ?

Jonathan Yap April 13, 2011, 9:19 a.m. (April 13, 2011, 9:19 a.m.)

Vadim's RB comments and other LL code implied this was the way to go.  I've just eliminated the extra public: line as it's only for one item.

indra/newview/llviewerchat.h (Diff revisions 2 - 3)

Indented with 4 spaces instead of 1 tab like the other lines.

Jonathan Yap April 13, 2011, 9:19 a.m. (April 13, 2011, 9:19 a.m.)
```
Fixed.
```

doc/contributions.txt (Diff revision 3)

I guess the STORM-1077 and STORM-1019 entries were added in error? Or why are you replacing them, rather than just adding STORM-1095?

Jonathan Yap April 13, 2011, 9:22 a.m. (April 13, 2011, 9:22 a.m.)

I am not the best Merge Monkey and must have made an error when merging these changesets into viewer-development when the MM sent this back.

Thanks, Jonathan. No major issues this time.

I've got a few comments though, in addition to what Boroondas said:

indra/newview/llbottomtray.cpp (Diff revision 3)

Any reason not to move this to LLNearbyChatBar::postBuild() ?

Jonathan Yap April 13, 2011, 12:36 p.m. (April 13, 2011, 12:36 p.m.)

Yes, there is a reason, though not necessarily a good one -- the same change was made for Storm-1094 which would probably have the merge monkey send it back for me to re-merge.  Since I cannot compile with autobuild I  would not be able to test this change and thus it would languish for a month+ until I have time to work on getting my autobuild issue resolved.

Vadim ProductEngine April 13, 2011, 1:14 p.m. (April 13, 2011, 1:14 p.m.)
```
I could do the merge for you.
```

Jonathan Yap April 13, 2011, 1:25 p.m. (April 13, 2011, 1:25 p.m.)

The bitbucket repo for this is current -- if you want to make this change and test it that would be wonderful.

Vadim ProductEngine April 13, 2011, 3 p.m. (April 13, 2011, 3 p.m.)
```
Done.

I'm closing this review request as submitted.
```

indra/newview/llnearbychatbar.h (Diff revision 3)

again the redundant change

Jonathan Yap April 13, 2011, 12:37 p.m. (April 13, 2011, 12:37 p.m.)
```
I don't know why hg diff is picking up this blank line.
```

indra/newview/llnearbychatbar.cpp (Diff revision 3)

RB seems to be set up to ignore whitespace changes and thus doesn't show this... but you've removed the leading tab here.

Please avoid irrelevant changes.

Jonathan Yap April 13, 2011, 12:38 p.m. (April 13, 2011, 12:38 p.m.)
```
Fixed.  Thanks for catching this.
```

indra/newview/llviewerchat.h (Diff revision 3)

Replace the redundant "public:" marker with an empty line.

Jonathan Yap April 13, 2011, 12:39 p.m. (April 13, 2011, 12:39 p.m.)

Also fixed, though in other LL code, where there are larger blocks to organize, multiple public: lines are found.

indra/newview/llviewerchat.h (Diff revision 3)

CS: remove spaces near parenthesis.

Jonathan Yap April 13, 2011, 12:39 p.m. (April 13, 2011, 12:39 p.m.)
```
Done.
```

indra/newview/llviewerchat.cpp (Diff revision 3)

CS: extra spaces near parenthesis

Jonathan Yap April 13, 2011, 12:39 p.m. (April 13, 2011, 12:39 p.m.)
```
Done.
```

Diff
- added Diff r4

Description:

Apply some formatting changes suggested in the RB comments.

doc/contributions.txt (Diff revision 4)

fix this before committing

Jonathan Yap April 13, 2011, 1:24 p.m. (April 13, 2011, 1:24 p.m.)
```
Fixed
```

doc/contributions.txt (Diff revision 4)

ditto

Jonathan Yap April 13, 2011, 1:24 p.m. (April 13, 2011, 1:24 p.m.)
```
Fixed
```

indra/newview/llviewerchat.cpp (Diff revision 4)

ditto

Jonathan Yap April 13, 2011, 1:24 p.m. (April 13, 2011, 1:24 p.m.)
```
Fixed.
```

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.