Added readonly mode to the CInput, useful for replays and other places with some system information, because it allows to copy the text without changes.
Details
Details
- Reviewers
elexis - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Owners Package (Owns No Changed Paths)
- Run game
- Open replays
- Select any replay
- Try to change the text
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 2680 Build 4570: Vulcan Build Jenkins
Event Timeline
Comment Actions
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1789/
See console output for more information: http://jw:8080/job/phabricator/1789/console
Comment Actions
0ad-project/binaries/... That's not going to apply cleanly when someone tries to test your patch. Please either use arc (described in https://trac.wildfiregames.com/wiki/Phabricator#UsingArcanist), or at the very least ensure that you create the patch using the directory that has binaries/ and source/ as the root.
Comment Actions
Oh, thank you! I didn't notice it, patcher selected wrong root, I'll fix it soon. Arc isn't good for me, because it crashes (exceptions) very often.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1791/ for more details.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1792/ for more details.
Comment Actions
I think you should disable the cursor or allow using arrow keys to navigate and mark text.
Else looks very nice ;)
(Hoping the next thing while be placeholder text ;P)
Comment Actions
Rest works as advertized (including changing the "readonly" property from JS) and ok with me. Nice patch!
| binaries/data/mods/public/gui/replaymenu/replay_menu.js | ||
|---|---|---|
| 274 ↗ | (On Diff #2996) | Why was escapeText removed? Setting caption = "foo\bar" shows an unprintable character here, with and without escapeText. Should be tested on windows. |
| source/gui/CInput.cpp | ||
| 544 ↗ | (On Diff #2996) | No else after return, but \n |
| source/gui/CInput.h | ||
| 178 ↗ | (On Diff #2996) | // Something |
Comment Actions
I'll disable the cursor.
(Hoping the next thing while be placeholder text ;P)
I want to make a placeholder too.
| binaries/data/mods/public/gui/replaymenu/replay_menu.js | ||
|---|---|---|
| 274 ↗ | (On Diff #2996) | It's tested and it works. Because the input shows the raw string as is. |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1835/ for more details.
Comment Actions
I'm not sure about the cursor.
Readonly should mean that we can do everything except writing to the element.
That includes navigating with arrow keys, home, end, super+arrow keys, skipping words with control and selecting text with shift+arrowkeys.
That is possible with writable elements, so why should it be impossible with readonly ones?
Mostly finding this odd because it's possible to select only parts of the word with the mouse, so the cursor position is still noticeable, it's just hidden.
On windows and common browsers, the behavior is the same, for example:
https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_input_readonly
You can hide the cursor if you want, but I'd keep the readonly operations accessible.
@Imarok agrees?
| binaries/data/mods/public/gui/replaymenu/replay_menu.js | ||
|---|---|---|
| 274 ↗ | (On Diff #2996) | According to Stan it works on windows without the escapeText: E:\Users\dolci\Documents\My Games\0ad\replays\0.0.23\2017-08-08_0001 As discussed on irc with Vladislav, removing the escapeText from the "text" elements doesn't work. So the change is correct and complete. |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1914/ for more details.
Comment Actions
Build was aborted
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)...............io.cpp(58): Function call failed: return value was -1 (Function failed (no details available)) Function call failed: return value was -1 (Function failed (no details available)) Location: io.cpp:58 (Issue) Call stack: (0xb8d70e) ./test() [0xb8d70e] (0xb1a9c9) ./test() [0xb1a9c9] (0xb1c8aa) ./test() [0xb1c8aa] (0xb32b47) ./test() [0xb32b47] (0xba95ae) ./test() [0xba95ae] (0xbac3fc) ./test() [0xbac3fc] (0xb380d8) ./test() [0xb380d8] (0x825ea9) ./test() [0x825ea9] (0x827ae4) ./test() [0x827ae4] (0x9dce47) ./test() [0x9dce47] (0x8c9519) ./test() [0x8c9519] (0x8ca4eb) ./test() [0x8ca4eb] (0xb55621) ./test() [0xb55621] (0x8cbb53) ./test() [0x8cbb53] (0x483cdf) ./test() [0x483cdf] (0x449595) ./test() [0x449595] errno = 0 (?) OS error = ? (C)ontinue, (S)uppress, (B)reak, Launch (D)ebugger, or (E)xit? Sleeping until debugger attaches. Please wait. Debugger launch failed: No such file or directory
Link to build: http://jw:8080/job/phabricator/1916/
See console output for more information: http://jw:8080/job/phabricator/1916/console
Comment Actions
- Ok for me to use both Immutable and Readonly phrases in the patch.
- argument instead of copy&paste:
// Since the GUI framework doesn't handle to set settings // in Unicode (CStrW), we'll simply retrieve the actual // pointer and edit that. CStrW* pCaption = (CStrW*)m_Settings["caption"].m_pSetting;
- const int szChar -> SDL_KeyCode
- Ok to not return IN_HANDLED in the two new functions, because that value should be returned in any case (because even if we press unrecognized characters, other GUI objects shouldn't be influenced).
- SDLK_DOWN and SDLK_UP are almost completely copies, but I don't care to unify.
- Not relevant code-wise, but seems a tiny bit more readable with scopes in all cases.
| source/gui/CInput.cpp | ||
|---|---|---|
| 221 ↗ | (On Diff #3317) | Don't really need the \t comment, nor the autocomplete one, nor the "let JS figure it out comment" |
| 223 ↗ | (On Diff #3317) | Since you included some whitespace fixes, then we can fix the different comment formats here as well. |
| 243 ↗ | (On Diff #3317) | wrong indentation |
| 513 ↗ | (On Diff #3317) | same amount as tabs as the if ( has, then 4 spaces |
| 552 ↗ | (On Diff #3317) | Re `/* END: Message History Lookup */`, there had been history lookup at some time in this file but it was deleted. So the comment is no good and should be deleted. |
| 567 ↗ | (On Diff #3317) | Preferable to not hardcode a list here but let the if-chain handle that. |
| 265 ↗ | (On Diff #3314) | Wat? (rP1904) |
| source/gui/CInput.h | ||
| 1 ↗ | (On Diff #3317) | bump |
Comment Actions
if u now get text color tags to cinput we could have select/copyable chat text window in lobby
Comment Actions
There is some source/gui/ function to remove GUI tags - or one to get the non-formatted text as far as I know.
However I wonder if copy&pasting lobby or ingame chat will be desirable feature.