← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/multiline_textarea into lp:widelands

 

Review: Approve

Sorry, I am late to the review-party. I try to do one code review per day these days - not more. But I do not always manage.

Just one minor style fix, I suggest to just do it in any other branch you are working on.

Otherwise this is an awesome change. The font handler is really coming together now.

Diff comments:

> 
> === modified file 'src/graphic/text_layout.cc'
> --- src/graphic/text_layout.cc	2015-12-13 18:32:28 +0000
> +++ src/graphic/text_layout.cc	2016-01-26 07:50:51 +0000
> @@ -54,12 +54,24 @@
>  	f % txt;
>  	return f.str();
>  }
> +
>  std::string as_uifont(const std::string & txt, int size, const RGBColor& clr) {
> +	return as_aligned(txt, UI::Align::Align_Left, size, clr);
> +}
> +
> +std::string as_aligned(const std::string & txt, UI::Align align, int ptsize, const RGBColor& clr) {

nit: space between std::string and &

> +	std::string alignment = "left";
> +	if ((align & UI::Align_Horizontal) == UI::Align::Align_Right) {
> +		alignment = "right";
> +	} else if ((align & UI::Align_Horizontal) == UI::Align::Align_HCenter) {
> +		alignment = "center";
> +	}
> +
>  	// UI Text is always bold due to historic reasons
>  	static boost::format
> -			f("<rt><p><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");
> -
> -	f % size;
> +			f("<rt><p align=%s><font face=serif size=%i bold=1 shadow=1 color=%s>%s</font></p></rt>");
> +	f % alignment;
> +	f % ptsize;
>  	f % clr.hex_value();
>  	f % txt;
>  	return f.str();


-- 
https://code.launchpad.net/~widelands-dev/widelands/multiline_textarea/+merge/283736
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/multiline_textarea.


References