Add text length limit on TextBox control #51
Conversation
Fair comment, but |
| @@ -63,6 +63,9 @@ namespace Gwk | |||
| virtual void SetCursorPos(int i); | |||
| virtual void SetCursorEnd(int i); | |||
|
|
|||
| void SetMaxTextLength(int maxLength) { m_maxTextLength = maxLength; } | |||
| const int& GetMaxTextLength() const { return m_maxTextLength; } | |||
| @@ -46,6 +46,7 @@ GWK_CONTROL_CONSTRUCTOR(TextBox) | |||
| m_cursorLine = 0; | |||
| m_bEditable = true; | |||
| m_bSelectAll = false; | |||
| m_maxTextLength = -1; | |||
There was a problem hiding this comment.
Be nice if -1 wasn't a magic number. Perhaps static constexpr NO_MAX_LENGTH = -1; ?
| { | ||
| Gwk::Controls::TextBoxMultiline* label = new Gwk::Controls::TextBoxMultiline(this); | ||
| label->SetText("Max Length 20"); | ||
| label->SetMaxTextLength(20); |
There was a problem hiding this comment.
After this, perhaps you could set the text to something longer than the maximum length to test whether this works as expected?
|
What happens is the text will be larger (since we're only checking the limit on insert), but nothing bad happens, the user just has to take the text below the limit before they can edit it. That seems alright to me. I'll add a test for it anyway |
Changed return type to int. Added a constant to replace magic number. Added a test covering longer than max length.
|
Yep, but I think you added another control, so just want to distinguish what the control is demonstrating. E.g. setText(...), setMaxTextLen(6), setText("Maximum text length"), so we can see that the length has been clamped/truncated, and that any editing maintains this. - possibly you could add a counter next to it showing the text length, but that is entirely optional. |
| @@ -90,6 +90,8 @@ namespace Gwk | |||
| Event::Caller onTextChanged; | |||
| Event::Caller onReturnPressed; | |||
|
|
|||
| static constexpr int NO_MAX_LENGTH = -1; | |||
There was a problem hiding this comment.
This could just be static const int NO_MAX_LENGTH = -1;
There was a problem hiding this comment.
This project is C++11 so there is no reason not to use constexpr.
|
Alright, changed it to add an extra SetText after SetMaxTextLength |
| @@ -80,7 +80,7 @@ void TextBox::InsertText(const Gwk::String& strInsert) | |||
| return; | |||
|
|
|||
| int insertSize = static_cast<int>(strInsert.size()); | |||
| if (m_maxTextLength > -1 && TextLength() + insertSize > m_maxTextLength) | |||
| if (m_maxTextLength > NO_MAX_LENGTH && TextLength() + insertSize > m_maxTextLength) | |||
There was a problem hiding this comment.
This should be != NO_MAX_LENGTH as it is a magic number out of the usual range.
Add text length limit on TextBox control
As per the discussion, went with an approach that once the limit is reached no more text can be entered. This seems to be the standard behaviour used by other applications and makes sense.
Can adjust naming/stuff if required.