widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

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

Requested reviews:
  Widelands Developers (widelands-dev)

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

Cleaned up buggy text positioning and width for spinboxes. Also, spinboxes now own their labels, just like checkboxes.

Labels will automatically take up more than 1 line if needed - we don't have a test case for this right now, but it safeguards us against text overflow with translations.

For testing: Spinboxes are only used in the Options window so far.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/spinbox into lp:widelands.

=== modified file 'src/ui_basic/spinbox.cc'
--- src/ui_basic/spinbox.cc	2015-10-23 10:17:13 +0000
+++ src/ui_basic/spinbox.cc	2015-12-01 17:08:30 +0000
@@ -29,6 +29,7 @@
 #include "graphic/text/font_set.h"
 #include "graphic/text_constants.h"
 #include "ui_basic/button.h"
+#include "ui_basic/multilinetextarea.h"
 #include "ui_basic/textarea.h"
 
 namespace UI {
@@ -76,13 +77,14 @@
  */
 SpinBox::SpinBox
 	(Panel * const parent,
-	 const int32_t x, const int32_t y, const uint32_t w,
+	 const int32_t x, const int32_t y, const uint32_t w, const uint32_t unit_w,
 	 int32_t const startval, int32_t const minval, int32_t const maxval,
+	 const std::string& label_text,
 	 const std::string& unit,
 	 const Image* background,
 	 bool const big)
 	:
-	Panel(parent, x, y, w, 0),
+	Panel(parent, x, y, std::max(w, unit_w), 0),
 	big_(big),
 	sbi_(new SpinBoxImpl)
 {
@@ -93,17 +95,40 @@
 	sbi_->background = background;
 
 	uint32_t padding = 2;
-
+	uint32_t actual_w = std::max(w, unit_w);
+	uint32_t no_padding = (big_ ? 6 : 4);
 	uint32_t texth = UI::g_fh1->render(as_uifont("."))->height();
-	box_ = new UI::Box(this, 0, 0, UI::Box::Horizontal, w, texth, padding);
+
+	// 40 is an ad hoc width estimate for the MultilineTextarea scrollbar + a bit of text.
+	if (!label_text.empty() && (w + padding) <= unit_w - 40) {
+		throw wexception(
+					"SpinBox: Overall width %d must be bigger than unit width %d + %d * %d + 40 for padding",
+					w, unit_w, no_padding, padding);
+	}
 
 #ifndef NDEBUG //  only in debug builds
-	if (w < (big_ ? 7 * texth : 3 * texth)) {
+	if (unit_w < (big_ ? 7 * texth : 3 * texth)) {
 		throw wexception("Not enough space to draw spinbox. Width %d is smaller than required width %d",
-							  w, (big_ ? 7 * texth : 3 * texth));
+							  unit_w, (big_ ? 7 * texth : 3 * texth));
 	}
 #endif
 
+	box_ = new UI::Box(this, 0, 0, UI::Box::Horizontal, actual_w, texth, padding);
+
+	// Find out how much height we need for the label. We give it 6 rows maximum.
+	const Image* rendered_text = UI::g_fh1->render(as_uifont(label_text));
+	uint32_t available_width = w - unit_w - no_padding * padding;
+	uint32_t extra_rows =
+			available_width > 0 ?
+				std::min(static_cast<int>(rendered_text->width() / available_width), 6) : 0;
+
+	UI::MultilineTextarea* label = new UI::MultilineTextarea(box_, 0, 0, available_width,
+																				texth * (extra_rows + 1), label_text);
+
+	box_->add(label, UI::Box::AlignTop);
+
+	sbi_->text = new UI::Textarea(box_, "", Align_Center);
+
 	sbi_->button_minus =
 		new Button
 			(box_, "-",
@@ -142,24 +167,22 @@
 		buttons_.push_back(sbi_->button_ten_minus);
 		buttons_.push_back(sbi_->button_ten_plus);
 
-		sbi_->text =
-				new UI::Textarea(
-					box_, 0, 0,
-					w - 2 * sbi_->button_ten_plus->get_w() - 2 * sbi_->button_minus->get_w() - 4 * padding, texth,
-					"", Align_Center);
+		sbi_->text->set_fixed_width(unit_w
+											 - 2 * sbi_->button_ten_plus->get_w()
+											 - 2 * sbi_->button_minus->get_w()
+											 - 4 * padding);
 
-		box_->add(sbi_->button_ten_minus, UI::Box::AlignCenter);
-		box_->add(sbi_->button_minus, UI::Box::AlignCenter);
-		box_->add(sbi_->text, UI::Box::AlignCenter);
-		box_->add(sbi_->button_plus, UI::Box::AlignCenter);
-		box_->add(sbi_->button_ten_plus, UI::Box::AlignCenter);
+		box_->add(sbi_->button_ten_minus, UI::Box::AlignTop);
+		box_->add(sbi_->button_minus, UI::Box::AlignTop);
+		box_->add(sbi_->text, UI::Box::AlignTop);
+		box_->add(sbi_->button_plus, UI::Box::AlignTop);
+		box_->add(sbi_->button_ten_plus, UI::Box::AlignTop);
 	} else {
-		sbi_->text = new UI::Textarea(box_, 0, 0,
-												w - 2 * sbi_->button_minus->get_w() - 2 * padding, texth,
-												"", Align_Center);
-		box_->add(sbi_->button_minus, UI::Box::AlignCenter);
-		box_->add(sbi_->text, UI::Box::AlignCenter);
-		box_->add(sbi_->button_plus, UI::Box::AlignCenter);
+		sbi_->text->set_fixed_width(unit_w - 2 * sbi_->button_minus->get_w() - 2 * padding);
+
+		box_->add(sbi_->button_minus, UI::Box::AlignTop);
+		box_->add(sbi_->text, UI::Box::AlignTop);
+		box_->add(sbi_->button_plus, UI::Box::AlignTop);
 	}
 
 	sbi_->button_plus->sigclicked.connect(boost::bind(&SpinBox::change_value, boost::ref(*this), 1));
@@ -168,8 +191,8 @@
 	sbi_->button_minus->set_repeating(true);
 	buttons_.push_back(sbi_->button_minus);
 	buttons_.push_back(sbi_->button_plus);
-	box_->set_size(w, texth);
-	set_size(w, texth);
+	box_->set_size(actual_w, texth * (extra_rows + 1));
+	set_size(actual_w, texth * (extra_rows + 1));
 	update();
 }
 

=== modified file 'src/ui_basic/spinbox.h'
--- src/ui_basic/spinbox.h	2015-10-23 10:17:13 +0000
+++ src/ui_basic/spinbox.h	2015-12-01 17:08:30 +0000
@@ -35,12 +35,16 @@
 struct TextStyle;
 
 /// A spinbox is an UI element for setting the integer value of a variable.
+/// w is the overall width of the SpinBox and must be wide enough to fit 2 labels and the buttons.
+/// unit_w is the width alotted for all buttons and the text between them (the actual spinbox).
+/// label_text is a text that precedes the actual spinbox.
 class SpinBox : public Panel {
 public:
 	SpinBox
 		(Panel*,
-		 int32_t x, int32_t y, uint32_t w,
+		 int32_t x, int32_t y, uint32_t w, uint32_t unit_w,
 		 int32_t startval, int32_t minval, int32_t maxval,
+		 const std::string& label_text = std::string(),
 		 const std::string& unit = std::string(),
 		 const Image* buttonbackground = g_gr->images().get("pics/but3.png"),
 		 bool big = false);

=== modified file 'src/ui_basic/textarea.cc'
--- src/ui_basic/textarea.cc	2015-11-22 08:20:56 +0000
+++ src/ui_basic/textarea.cc	2015-12-01 17:08:30 +0000
@@ -80,6 +80,7 @@
  */
 void Textarea::init()
 {
+	fixed_width_ = 0;
 	set_handle_mouse(false);
 	set_thinks(false);
 	set_textstyle(UI::TextStyle::ui_small());
@@ -147,6 +148,14 @@
 
 
 /**
+ * Set the fixed width. The Textarea will still collapse, but then restore this width when expand() is called.
+ */
+void Textarea::set_fixed_width(uint32_t w) {
+	fixed_width_ = w;
+}
+
+
+/**
  * Redraw the Textarea
  */
 void Textarea::draw(RenderTarget & dst)
@@ -223,7 +232,7 @@
 	uint16_t h = 0;
 
 	if (rendered_text_) {
-		w = rendered_text_->width();
+		w = fixed_width_ > 0 ? fixed_width_ : rendered_text_->width();
 		h = rendered_text_->height();
 		// We want empty textareas to have height
 		if (m_text.empty()) {

=== modified file 'src/ui_basic/textarea.h'
--- src/ui_basic/textarea.h	2015-11-22 08:20:56 +0000
+++ src/ui_basic/textarea.h	2015-12-01 17:08:30 +0000
@@ -68,7 +68,13 @@
 		 const std::string & text = std::string(),
 		 Align align = Align_Left);
 
-	void set_fixed_size(const std::string & text);
+	/**
+	 * If fixed_width > 0, the Textarea will not change its width.
+	 * Use this if you need a Textarea that keeps changing its contents, but you don't want the
+	 * surrounding elements to shift, e.g. in a Box.
+	 */
+	void set_fixed_width(uint32_t w);
+
 	void set_text(const std::string &);
 	const std::string& get_text();
 
@@ -99,6 +105,7 @@
 	const Image* rendered_text_;
 	Align m_align;
 	UI::TextStyle m_textstyle;
+	uint32_t fixed_width_;
 };
 
 }

=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc	2015-10-10 20:45:27 +0000
+++ src/ui_fsmenu/options.cc	2015-12-01 17:08:30 +0000
@@ -150,16 +150,12 @@
 									 m_fullscreen.get_y() +
 									 m_fullscreen.get_h() + m_padding),
 					 _("Grab Input")),
-	m_label_maxfps
+	m_sb_maxfps
 		(this,
 		 m_hmargin,
 		 m_inputgrab.get_y() + m_inputgrab.get_h() + m_padding,
-		 m_reslist.get_w() - 105, m_inputgrab.get_h(),
-		 _("Maximum FPS:"), UI::Align_VCenter),
-	m_sb_maxfps
-		(this,
-		 m_hmargin + m_reslist.get_w() - 105, m_label_maxfps.get_y(), 105,
-		 opt.maxfps, 0, 99, ""),
+		 m_reslist.get_w(), 105,
+		 opt.maxfps, 0, 99, _("Maximum FPS:"), ""),
 
 
 	// First options block 'general options', second column
@@ -216,38 +212,29 @@
 									 _("Dock windows to edges")),
 	m_sb_autosave
 		(this,
-		 get_w() - m_hmargin - 240,
+		 m_hmargin,
 		 m_dock_windows_to_edges.get_y() + m_dock_windows_to_edges.get_h() + m_padding,
+		 get_w() - 2 * m_hmargin,
 		 240,
+		 opt.autosave / 60, 0, 100,
+		 _("Save game automatically every"),
 		 /** TRANSLATORS: Options: Save game automatically every: */
 		 /** TRANSLATORS: This will have a number added in front of it */
-		 opt.autosave / 60, 0, 100, ngettext("minute", "minutes", opt.autosave / 60),
+		 ngettext("minute", "minutes", opt.autosave / 60),
 		 g_gr->images().get("pics/but3.png"), true),
-	m_label_autosave
-		(this,
-		 m_dock_windows_to_edges.get_x(),
-		 m_sb_autosave.get_y(),
-		 get_w() - m_sb_autosave.get_w() - 2 * m_hmargin,
-		 m_dock_windows_to_edges.get_h(),
-		 _("Save game automatically every"), UI::Align_VCenter),
 
 	m_sb_remove_replays
 		(this,
-		 get_w() - m_hmargin - 240,
+		 m_hmargin,
 		 m_sb_autosave.get_y() + m_sb_autosave.get_h() + m_padding,
+		 get_w() - 2 * m_hmargin,
 		 240,
+		 opt.remove_replays, 0, 365,
+		 _("Remove replays older than:"),
 		 /** TRANSLATORS: Options: Remove Replays older than: */
 		 /** TRANSLATORS: This will have a number added in front of it */
-		 opt.remove_replays, 0, 365, ngettext("day", "days", opt.remove_replays),
+		 ngettext("day", "days", opt.remove_replays),
 		 g_gr->images().get("pics/but3.png"), true),
-	m_label_remove_replays
-		(this,
-		 m_label_autosave.get_x(),
-		 m_sb_remove_replays.get_y(),
-		 get_w() - m_sb_remove_replays.get_w() - 2 * m_hmargin,
-		 m_dock_windows_to_edges.get_h(),
-		 _("Remove replays older than:"), UI::Align_VCenter),
-
 	os(opt)
 {
 	m_advanced_options.sigclicked.connect
@@ -469,30 +456,26 @@
 		 get_w() / 2, get_h() * 17 / 150,
 		 _("Advanced Options"), UI::Align_HCenter),
 
-// First options block
-	m_label_snap_dis_panel
-		(this,
-		 m_hmargin, get_h() * 9 / 30,
-		 _("Distance for windows to snap to other panels:"), UI::Align_VCenter),
-	m_label_snap_dis_border
-		(this,
-		 m_hmargin,  m_label_snap_dis_panel.get_y() + m_label_snap_dis_panel.get_h() + 2 * m_padding,
-		 _("Distance for windows to snap to borders:"), UI::Align_VCenter),
-
 	// Spinboxes
 	m_sb_dis_panel
 			(this,
-			 get_w() - m_hmargin - (get_w() / 5), m_label_snap_dis_panel.get_y(), get_w() / 5,
-			 opt.panel_snap_distance, 0, 99, ngettext("pixel", "pixels", opt.panel_snap_distance)),
+			 m_hmargin, get_h() * 9 / 30,
+			 get_w() - 2 * m_hmargin, get_w() / 5,
+			 opt.panel_snap_distance, 0, 99,
+			 _("Distance for windows to snap to other panels:"),
+			 ngettext("pixel", "pixels", opt.panel_snap_distance)),
 
 	m_sb_dis_border
 			(this,
-			 get_w() - m_hmargin - (get_w() / 5), m_label_snap_dis_border.get_y(), get_w() / 5,
-			 opt.border_snap_distance, 0, 99, ngettext("pixel", "pixels", opt.border_snap_distance)),
+			 m_hmargin, m_sb_dis_panel.get_y() + m_sb_dis_panel.get_h() + 2 * m_padding,
+			 get_w() - 2 * m_hmargin, get_w() / 5,
+			 opt.border_snap_distance, 0, 99,
+			 _("Distance for windows to snap to borders:"),
+			 ngettext("pixel", "pixels", opt.border_snap_distance)),
 
 	m_transparent_chat (this, Point(m_hmargin,
-											  m_label_snap_dis_border.get_y() +
-											  m_label_snap_dis_border.get_h() + m_space),
+											  m_sb_dis_border.get_y() +
+											  m_sb_dis_border.get_h() + m_space),
 							  _("Show in-game chat with transparent background"),
 							  "", get_w() - 2 * m_hmargin),
 

=== modified file 'src/ui_fsmenu/options.h'
--- src/ui_fsmenu/options.h	2015-10-09 18:06:41 +0000
+++ src/ui_fsmenu/options.h	2015-12-01 17:08:30 +0000
@@ -102,7 +102,6 @@
 	UI::Listselect<void *>      m_reslist;
 	UI::Checkbox                m_fullscreen;
 	UI::Checkbox                m_inputgrab;
-	UI::Textarea                m_label_maxfps;
 	UI::SpinBox                 m_sb_maxfps;
 
 	UI::Textarea                m_label_language;
@@ -117,9 +116,7 @@
 	UI::Checkbox                m_snap_win_overlap_only;
 	UI::Checkbox                m_dock_windows_to_edges;
 	UI::SpinBox                 m_sb_autosave;
-	UI::Textarea                m_label_autosave;
 	UI::SpinBox                 m_sb_remove_replays;
-	UI::Textarea                m_label_remove_replays;
 
 	OptionsCtrl::OptionsStruct  os;
 
@@ -164,7 +161,6 @@
 	UI::Button                  m_cancel, m_apply;
 	UI::Textarea                m_title;
 
-	UI::Textarea                m_label_snap_dis_panel, m_label_snap_dis_border;
 	UI::SpinBox                 m_sb_dis_panel, m_sb_dis_border;
 	UI::Checkbox                m_transparent_chat;
 	UI::Checkbox                m_message_sound;