widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/messagebox into lp:widelands.

Commit message:
Refactored WLMessageBox. It now no longer depends on the old font renderer, and scrollbars only appear when they are needed.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/messagebox/+merge/285023

Refactored WLMessageBox to get rid of using the old font renderer in it. The textarea will now only create a scrollbar if there is lots of text.

I also switched the button order. OK should always be on the right for consistency (and on the left for RTL languages).

I haven't used a box layout, because the buttons wouldn't go where they were supposed to go. I plan to go through all the message boxes in the future and implement consistent button ordering for OK/Cancel, so I will look into that again then - maybe implement a new class "button group".

I expect that the diff won't be of much use, best look at the changed files directly.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/messagebox into lp:widelands.

=== modified file 'src/ui_basic/messagebox.cc'
--- src/ui_basic/messagebox.cc	2016-01-28 05:24:34 +0000
+++ src/ui_basic/messagebox.cc	2016-02-04 09:08:39 +0000
@@ -20,110 +20,106 @@
 #include "ui_basic/messagebox.h"
 
 #include "base/i18n.h"
-#include "graphic/font_handler.h"
 #include "graphic/font_handler1.h"
 #include "graphic/graphic.h"
-#include "ui_basic/button.h"
-#include "ui_basic/multilinetextarea.h"
 #include "ui_basic/window.h"
 
 namespace UI {
 
-
-struct WLMessageBoxImpl {
-	MultilineTextarea * textarea;
-	WLMessageBox::MBoxType type;
-};
-
-
-WLMessageBox::WLMessageBox
-	(Panel * const parent,
-	 const std::string & caption,
-	 const std::string & text,
-	 const MBoxType type,
-	 Align align)
-	:
-	Window(parent, "message_box", 0, 0, 20, 20, caption.c_str()),
-	d(new WLMessageBoxImpl)
-{
-	d->type = type;
-
-	const int32_t outerwidth = parent ?
-		parent->get_inner_w() : g_gr->get_xres();
-	const int32_t outerheight = parent ?
-		parent->get_inner_h() : g_gr->get_yres();
-	assert(outerwidth >= 80);
-	assert(outerheight >= 60);
-	const int32_t maxwidth = outerwidth - 80;
-	const int32_t maxheight = outerheight - 60;
-	d->textarea = new MultilineTextarea
-		(this,
-		 5, 5, maxwidth, maxheight,
-		 text.c_str(), align);
-
-	uint32_t width, height;
-	std::string font = UI::g_fh1->fontset().serif();
-	int32_t fontsize = UI_FONT_SIZE_SMALL;
-
-	UI::g_fh->get_size(font, fontsize, text, width, height, maxwidth);
-	// stupid heuristic to avoid excessively long lines
-	if (height < 2U * fontsize)
-		UI::g_fh->get_size(font, fontsize, text, width, height, maxwidth / 2);
-
-	width += 10 + 2 * d->textarea->scrollbar_w();
-	if (width < 100)
-		width = 100;
-	if (width > static_cast<uint32_t>(maxwidth))
-		width = maxwidth;
-	height += 50;
-	if (height > static_cast<uint32_t>(maxheight))
-		height = maxheight;
-
-	set_inner_size(width, height);
-	set_pos(Point((outerwidth - get_w()) / 2, (outerheight - get_h()) / 2));
-
-	d->textarea->set_size(width - 10, height - 50);
-
-	if (type == MBoxType::kOk) {
-		UI::Button * okbtn = new Button
-			(this, "ok",
-			 (get_inner_w() - 120) / 2, get_inner_h() - 30, 120, 20,
-			 g_gr->images().get("images/ui_basic/but5.png"),
-			 _("OK"));
-		okbtn->sigclicked.connect(boost::bind(&WLMessageBox::clicked_ok, boost::ref(*this)));
-	} else if (type == MBoxType::kOkCancel) {
-		UI::Button * okbtn = new Button
-			(this, "ok",
-			 (get_inner_w() / 2 - 120) / 2, get_inner_h() - 30, 120, 20,
-			 g_gr->images().get("images/ui_basic/but5.png"),
-			 _("OK"));
-		okbtn->sigclicked.connect(boost::bind(&WLMessageBox::clicked_ok, boost::ref(*this)));
-		UI::Button * cancelbtn = new Button
-			(this, "no",
-			 (get_inner_w() / 2 - 120) / 2 + get_inner_w() / 2, get_inner_h() - 30,
-			 120, 20,
-			 g_gr->images().get("images/ui_basic/but1.png"),
-			 _("Cancel"));
-		cancelbtn->sigclicked.connect(boost::bind(&WLMessageBox::clicked_back, boost::ref(*this)));
-	}
+WLMessageBox::WLMessageBox(Panel* const parent,
+                           const std::string& caption,
+                           const std::string& text,
+                           const MBoxType type,
+                           Align align)
+   : Window(parent, "message_box", 0, 0, 20, 20, caption.c_str()), type_(type) {
+	// Calculate textarea dimensions depending on text size
+	const int outerwidth = parent ? parent->get_inner_w() : g_gr->get_xres();
+	const int outerheight = parent ? parent->get_inner_h() : g_gr->get_yres();
+
+	const int button_w = 120;
+	const int minwidth = 3.5 * button_w;
+
+	// Ample space for the buttons, but not hugely wide
+	const int maxwidth = std::min(600, std::max(outerwidth * 2 / 3, minwidth));
+	// Make sure that there is space for buttons + message, but not too tall
+	const int maxheight = std::min(400, std::max(outerheight * 2 / 3, 200));
+
+	const int button_space = 50;
+	const int margin = 5;
+	int width, height = 0;
+	{
+		const Image* temp_rendered_text = g_fh1->render(as_uifont(text), maxwidth);
+		width = temp_rendered_text->width();
+		height = temp_rendered_text->height();
+	}
+
+	// Stupid heuristic to avoid excessively long lines
+	if (height < 2 * UI_FONT_SIZE_SMALL) {
+		const Image* temp_rendered_text = g_fh1->render(as_uifont(text), maxwidth / 2);
+		width = temp_rendered_text->width();
+		height = temp_rendered_text->height();
+	}
+
+	// Make sure that the buttons really fit
+	width = std::max(std::min(width, maxwidth), minwidth);
+	height += button_space;
+
+	// Find out whether the textarea needs a scrollbar
+	MultilineTextarea::ScrollMode scrollmode = MultilineTextarea::ScrollMode::kNoScrolling;
+	if (height > maxheight) {
+		height = maxheight - button_space;
+		scrollmode = MultilineTextarea::ScrollMode::kScrollNormal;
+	}
+
+	textarea_.reset(new MultilineTextarea(
+	   this, margin, margin, width - 2 * margin, height, text, align, scrollmode));
+
+	// Now add the buttons
+	const int button_y = textarea_->get_y() + textarea_->get_h() + 2 * margin;
+	const int left_button_x = width / 3 - button_w / 2;
+	const int right_button_x = width * 2 / 3 - button_w / 2;
+
+	ok_button_.reset(new Button(this,
+	                            "ok",
+	                            type_ == MBoxType::kOk ?
+	                               (width - button_w) / 2 :
+	                               UI::g_fh1->fontset().is_rtl() ? left_button_x : right_button_x,
+	                            button_y,
+	                            button_w,
+	                            0,
+	                            g_gr->images().get("images/ui_basic/but5.png"),
+	                            _("OK")));
+	ok_button_->sigclicked.connect(boost::bind(&WLMessageBox::clicked_ok, boost::ref(*this)));
+
+	if (type_ == MBoxType::kOkCancel) {
+		cancel_button_.reset(
+		   new Button(this,
+		              "no",
+		              UI::g_fh1->fontset().is_rtl() ? right_button_x : left_button_x,
+		              button_y,
+		              button_w,
+		              0,
+		              g_gr->images().get("images/ui_basic/but1.png"),
+		              _("Cancel")));
+		cancel_button_->sigclicked.connect(
+		   boost::bind(&WLMessageBox::clicked_back, boost::ref(*this)));
+	}
+
+	set_inner_size(width, button_y + ok_button_->get_h() + margin);
+	center_to_parent();
 	focus();
 }
 
-WLMessageBox::~WLMessageBox()
-{
-}
-
 /**
  * Handle mouseclick.
  *
  * Clicking the right mouse button inside the window acts like pressing
  * Ok or No, depending on the message box type.
  */
-bool WLMessageBox::handle_mousepress(const uint8_t btn, int32_t, int32_t)
-{
+bool WLMessageBox::handle_mousepress(const uint8_t btn, int32_t, int32_t) {
 	if (btn == SDL_BUTTON_RIGHT) {
 		play_click();
-		if (d->type == MBoxType::kOk) {
+		if (type_ == MBoxType::kOk) {
 			clicked_ok();
 		} else {
 			clicked_back();
@@ -132,43 +128,40 @@
 	return true;
 }
 
-bool WLMessageBox::handle_mouserelease(const uint8_t, int32_t, int32_t)
-{
+bool WLMessageBox::handle_mouserelease(const uint8_t, int32_t, int32_t) {
 	return true;
 }
 
-bool WLMessageBox::handle_key(bool down, SDL_Keysym code)
-{
+bool WLMessageBox::handle_key(bool down, SDL_Keysym code) {
 	if (down) {
 		switch (code.sym) {
-			case SDLK_KP_ENTER:
-			case SDLK_RETURN:
+		case SDLK_KP_ENTER:
+		case SDLK_RETURN:
+			clicked_ok();
+			return true;
+		case SDLK_ESCAPE:
+			if (type_ == MBoxType::kOk) {
 				clicked_ok();
-				return true;
-			case SDLK_ESCAPE:
+			} else {
 				clicked_back();
-				return true;
-			default:
-				break; // not handled
+			}
+			return true;
+		default:
+			break;  // not handled
 		}
 	}
 	return UI::Panel::handle_key(down, code);
 }
 
-
-void WLMessageBox::clicked_ok()
-{
+void WLMessageBox::clicked_ok() {
 	ok();
 	if (is_modal())
 		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
 }
 
-void WLMessageBox::clicked_back()
-{
+void WLMessageBox::clicked_back() {
 	cancel();
 	if (is_modal())
 		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
 }
-
-
 }

=== modified file 'src/ui_basic/messagebox.h'
--- src/ui_basic/messagebox.h	2015-12-17 09:36:59 +0000
+++ src/ui_basic/messagebox.h	2016-02-04 09:08:39 +0000
@@ -25,48 +25,43 @@
 #include <boost/signals2.hpp>
 
 #include "graphic/align.h"
+#include "ui_basic/button.h"
+#include "ui_basic/multilinetextarea.h"
 #include "ui_basic/window.h"
 
 namespace UI {
 
-struct WLMessageBoxImpl;
-
 /**
  * Shows a standard messagebox. The message box can be used either as a modal
  * or as a non-modal dialog box.
  *
  * Using it as a modal dialog box is very straightforward:
- *     WLMessageBox mb(parent, "Caption", "Text", OK);
+ *     WLMessageBox mb(parent, "Title", "Text", MBoxType::kOK);
  *     UI::Panel::Returncodes code = mb.run<UI::Panel::Returncodes>();
- * The return code is ok_code if the "OK" button has been pressed in a \ref YESNO
- * dialog box. Otherwise, it is dying_code (or negative, if the modal messagebox has
+ * The return code is kOk if the "OK" button has been pressed in a \ref MBoxType::kOkCancel
+ * dialog box. Otherwise, it is kBack (or negative, if the modal messagebox has
  * been interrupted in an unusual way).
  *
  * Using it as a non-modal dialog box is slightly more complicated. You have
  * to add this dialog box as a child to the current fullscreen panel, and
- * connect the signals \ref ok and \ref no or \ref ok, depending on the
+ * connect the signals \ref ok and \ref cancel or \ref ok, depending on the
  * messagebox type, to a function that deletes the message box.
  * \note this function is named "WLMessageBox" instead of simply "MessageBox"
  *       because else linking on Windows (even with #undef MessageBox) will
  *       not work.
 */
 struct WLMessageBox : public Window {
-	enum class MBoxType {
-		kOk,
-		kOkCancel
-	};
-	WLMessageBox
-		(Panel * parent,
-		 const std::string & caption,
-		 const std::string & text,
-		 MBoxType,
-		 Align = UI::Align::kCenter);
-	~WLMessageBox();
-
-	boost::signals2::signal<void ()> ok;
-	boost::signals2::signal<void ()> cancel;
-
-	bool handle_mousepress  (uint8_t btn, int32_t mx, int32_t my) override;
+	enum class MBoxType { kOk, kOkCancel };
+	WLMessageBox(Panel* parent,
+	             const std::string& caption,
+	             const std::string& text,
+	             MBoxType,
+	             Align = UI::Align::kCenter);
+
+	boost::signals2::signal<void()> ok;
+	boost::signals2::signal<void()> cancel;
+
+	bool handle_mousepress(uint8_t btn, int32_t mx, int32_t my) override;
 	bool handle_mouserelease(uint8_t btn, int32_t mx, int32_t my) override;
 
 	/// Handle keypresses
@@ -77,9 +72,10 @@
 	virtual void clicked_back();
 
 private:
-	std::unique_ptr<WLMessageBoxImpl> d;
+	MBoxType type_;
+	std::unique_ptr<Button> ok_button_, cancel_button_;
+	std::unique_ptr<MultilineTextarea> textarea_;
 };
-
 }
 
 #endif  // end of include guard: WL_UI_BASIC_MESSAGEBOX_H