| [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #3305
Application: | FLTK Library |
Status: | 1 - Closed w/Resolution |
Priority: | 3 - Moderate, e.g. unable to compile the software |
Scope: | 3 - Applies to all machines and operating systems |
Subsystem: | Core Library |
Summary: | Fl_Text_Editor: add a method to support tab navigation |
Version: | 1.3-current |
Created By: | greg.ercolano |
Assigned To: | greg.ercolano |
Fix Version: | 1.3.4 (SVN: v11808) |
Update Notification: | |
Trouble Report Files:
Trouble Report Comments:
|
#1 | greg.ercolano 12:36 May 17, 2016 |
| Users trying to make a database form will find it hard to support tab navigation if they use an Fl_Text_Editor for multiline input for one of the fields, as tab is used as an input character.
1) There should be a method, similar to Fl_Input's tab_nav(0|1) to make it easy to use in this context.
2) Perhaps too, a new wrapper widget, e.g. Fl_Text_Input, that implements Fl_Text_Editor with this flag set in the ctor, so form users can simply use it instead.
3) Additionally, such a widget could maybe implement an Fl_Text_Buffer internally so that one doesn't have to make the separate buffer instance and buffer() assignment, making it "easy" to use, esp. in Fluid. (As it is, a new user would find it challenging to make an Fl_Text_Editor/Display in fluid, having to know in advance about not only the details of Fl_Text_Buffer, but how to do these multiple steps all within fluid)
#1 for sure, #2 maybe, #3 for debate. | |
|
#2 | ianmacarthur 01:51 May 18, 2016 |
| Point #1:
Note that I would contend that we already have #1 (well, sort of) though the API to access it is somewhat obscure.
The crux to this is that you can tell the Fl_Text_Editor (and its sibling Fl_Text_Display) specifically which keys to process, and what to do with them.
By default, most keys are set to the "default" key handler, and so are processed by the widget.
However, it is possible to set *any* key to use the "ignore" handler instead - this is what is done by default in the editor the widget for the ESC key, to ensure that fltk's usual ESC key handling happens and is not subverted by the editor widget. (And you can also define custom handlers too, should you be so inclined.)
It is fairly straightforward therefore (albeit cryptic) to redirect FL_Tab to the "ignore" handler, and the editor widget will then allow tab navigation "normally" since it will no longer grab the TAB key for itself.
For an editor widget called "edit_1" this would look like:
// Tell the editor to ignore tabs edit_1->add_key_binding (FL_Tab, FL_TEXT_EDITOR_ANY_STATE, Fl_Text_Editor::kf_ignore);
With that line implemented (probably where the widget is instantiated) you end up with an editor widget that will participate cleanly in TAB navigation but which loses the ability to type any TAB characters.
For the "forms entry" case outlined in Greg's note, this is probably sufficient.
A more complex case might alter the STATE for the key handler from FL_TEXT_EDITOR_ANY_STATE so that it handled some or all of TAB, ALT+TAB, SHIFT+TAB, CTRL+TAB differently, opening the prospect of the user being able to use a "bare" TAB for navigation, and (say) a SHIFT+TAB to enter a TAB character into the form.
Point #2:
Seems reasonable, as noted above it ought to be a fairly simple extension to the stock editor widget, just done in the constructor?
Point #3:
If #2, then I guess #3 is painless. However, for more complex editor uses there are advantages to managing the text buffer and the editor view independently. | |
|
#3 | ianmacarthur 01:59 May 18, 2016 |
| Posted example file:
editor-tab-navigation-1.cxx
illustrating an editor widget participating in TAB navigation "normally". | |
|
#4 | ianmacarthur 09:05 May 18, 2016 |
| Posted revised editor code that shows how to allow regular TAB navigation, but also allow the user to type CTRL+TAB into the editor widget, if they actually need to type a TAB, rather than just grabbing all TAB keystrokes as navigation keys.
Not the most intuitive way to type a TAB mind you, but I was just trying to illustrate the point.
File: editor-tab-navigation-2.cxx
Tested on WIN7 only; I assume it would work "anywhere" though. | |
|
#5 | greg.ercolano 09:38 May 18, 2016 |
| In Fl_Input, the docs say ^I can be used to insert a tab. We should probably support the same for consistency.. opinions?
I'm about to suggest a patch that uses your technique. Perhaps that patch should include having ^I default behavior to insert a tab?
Currently ^I isn't bound in Fl_Text_Editor, so it does nothing when typed.
However, our test/editor demo seems to overload it for inserting a file. Perhaps the editor demo should use Alt-I instead.
An aside: While writing the above patch, I realized a separate issue outlined in STR#3306. | |
|
#6 | greg.ercolano 10:24 May 18, 2016 |
| Adding Fl_Text_Editor-tab_nav-v1.patch which simply adds a tab_nav() set/get method. Defaults remain the same.
As implemented, this is actually ABI safe.
Advising tab_nav(void) be left non-const until STR 3306 is resolved, then ABI guards can be added so it can all be fixed properly in 1.4 | |
|
#7 | ianmacarthur 02:14 May 19, 2016 |
| Updated example code to also support "CTRL+i" for TAB insertion, as suggested by Greg based on what we document for the Fl_Input widget family.
File: editor-tab-navigation-3.cxx | |
|
#8 | ianmacarthur 02:30 May 19, 2016 |
| @Greg
I was looking at the Fl_Text_Editor-tab_nav-v1.patch file, and I think the comment here is awry:
+ \returns 1 if Tab advances focus (default), 0 if Tab inserts tab characters.
I think the default is the "Tab inserts tab characters" case, is it not?
Anyway, that got me wondering whether testing that bound_key_function() with FL_TEXT_EDITOR_ANY_STATE is bound to "kf_ignore", is really a robust way to test for this?
I'm thinking that a user might have encoded some set of TAB key combos, one of which matches "kf_ignore", and in that circumstance we would report that tab_nav() was asserted, when in fact it might not be?
Though I have not tested, so maybe this is OK as is?
I guess we would perhaps want to add a member variable to Fl_Text_Editor to explicitly record what the user had passed to tab_nav() and report that, giving us a "direct" way of assessing whether the user had requested tab_nav or not.
But I imagine that would be more of an issue at the ABI level than what you are proposing here. | |
|
#9 | greg.ercolano 08:33 May 19, 2016 |
| > I think the default is the "Tab inserts tab characters" case, > is it not?
Yes, thanks.
That was a copy/paste carryover from Fl_Input's tab_nav() (where it IS the default) which I forgot to modify.
> Anyway, that got me wondering whether testing that bound_key_function() > with FL_TEXT_EDITOR_ANY_STATE is bound to "kf_ignore", is really a > robust way to test for this? > > I'm thinking that a user might have encoded some set of TAB key combos, > one of which matches "kf_ignore", and in that circumstance we would > report that tab_nav() was asserted, when in fact it might not be?
Ah, do you mean changing the use of FL_TEXT_EDITOR_ANY_STATE to a different value, e.g. 0?
Could be -- we surely only want to affect Tab without modifiers. (Shift-Tab /already/ seems to be ignored, which is expected for Tab nav'ing in reverse, so I think we need to only focus on fwd-nav)
Or are you referring to the use of kf_ignore to test in a more general way, even with the above mods to only focus on Tab with no modifiers? I figure if a user messes around with the bindings for Tab, they shouldn't be surprised if the value of tab_nav() is affected. | |
|
#10 | greg.ercolano 08:36 May 19, 2016 |
| Oh, and a separate issue: the FL_TEXT_EDITOR_ANY_STATE value could use some docs; it appears to currently be undocumented.
I was confused about what it does, and see my use of it in tab_nav() is probably wrong, the value to use should probably be 0. And I may need to make a separate kf_ignore binding for Shift-Tab. Will do some tests. | |
|
#11 | greg.ercolano 08:57 May 19, 2016 |
| BTW, regarding ^I to insert tabs, I was thinking we should make that /default/ behavior for Fl_Text_Editor, to match Fl_Input + friend's default behavior.
In other words, an additional entry in the default_key_bindings[] definition in Fl_Text_Editor.cxx, something to the effect of:
.. { FL_Tab, FL_CTRL, Fl_Text_Editor::kf_tab }, ..
..where "kf_tab" would be a new function that simply inserts a tab character, with the same or similar code to what your ctrl_i_to_tab() function does in your editor-tab-navigation-3.cxx file.
Comments? | |
|
#12 | greg.ercolano 09:23 May 19, 2016 |
| Attaching v3 patch, which addresses these issues:
o Only binds/unbinds the *un-modified* Tab key o Docs fixed as per Ian's note about defaults o Docs simplified/clarified o Made the tab_nav(void) method const. This has the side effect of forcing this to be an ABI 1.3.4 feature, because the existing bound_key_function() method tab_nav() uses is itself not const. (The ABI restriction can be removed if we release tab_nav(void) as a non-const method) o Due to the ABI feature, added \version doxygen tags indicating this is a 1.3.4 feature only.
Not addressed: ^I default behavior inserting a tab. Want to get comments on that first, and would address that as a separate patch. | |
|
#13 | AlbrechtS 09:53 May 25, 2016 |
| Notes concerning Fl_Text_Editor-tab_nav-v3.patch:
(1) I tried to fix a few formatting issues (Doxygen docs) and a typo. The modified patch Fl_Text_Editor-tab_nav-v3a.patch is attached.
(2) I believe that these new methods are not (closely, really) related to Fl::OPTION_ARROW_FOCUS, so the two instances of "\see ... Fl::OPTION_ARROW_FOCUS." should probably be changed (remove Fl::OPTION_ARROW_FOCUS). What do you think?
(3) If we intend to change the implementation, then the new methods should better not be inlined (in the header file) but be moved to the .cxx file instead. Again, asking for opinions.
(4) Question: should we really add this to FLTK 1.3.4 (as the ABI guards in the proposed patch indicate), or should we better do it in the porting branch, i.e. 1.4.0? My opinion is that we should also address the ^I insertion and I'd prefer an implementation that does not interfere with key definitions (the mentioned implementation change?). If so, this would definitely have to go into 1.4.0, since that change would have potential to break something. But, as above, asking for opinions...
PS: The tab_nav() docs didn't show up in Doxygen because they were guarded by ABI guards which Doxygen didn't know about. I committed svn r11754 that fixes this (by defining FLTK_ABI_VERSION=99999 in Doxygen). | |
|
#14 | greg.ercolano 15:32 Jul 13, 2016 |
| @Albrecht:
#1 -- great.
#2 -- Yes, I'd agree, no reference to OPTION_ARROW_FOCUS needed; in this case they're unrelated issues.
#3 -- Yes, in general it seems best to keep code in the .cxx files unless the implementation is /really/ simple. My goal here was probably to just minimize the code deltas.
#4 -- This is an interesting issue.
I suppose we want ABI features to be documented regardless of what the ABI value was set to when the doxygen docs are built. (This way e.g. the website docs can reflect new features as well as old behavior)
Pretty sure in the past I made separate documentation for old vs. new ABI features, so the docs were only consistent with what was built.
But I suppose it might be best if the docs always reflected old and new behaviors, and making sure those docs don't change with the ABI version macro.
This means the docs need to clearly indicate old and new behavior, referring to specific ABI version numbers.
We might need to make a pass at that before releasing 1.3.4 and regenerating the website's docs. | |
|
#15 | greg.ercolano 10:33 Jul 14, 2016 |
| Related: I committed a fix to related patch STR #3306 which makes the bound_key_function() function const when the ABI is set >= 10304.
This addresses the problem mentioned in comment #6 below, so all patches here that try to deal with that issue are no longer needed. | |
|
#16 | greg.ercolano 12:02 Jul 14, 2016 |
| Fixed in Subversion repository.
Assigned to myself, committed as 11808, closed.
Notes on the commit:
> I used Albrecht's v3a patch as a model
> Took his suggestion re: moving the implementation into the .cxx (so that future changes to the code wouldn't break ABI by being inline)
The only reason this mod needed ABI guards at all is because of the const issue with the bound_key_function(), solved by related STR #3306. (Didn't want to add this new tab_nav() method as non-const)
Also attaching the test program I used to verify this new tab nav flag. I'd add this test to the test/unittests program, but it's such a PITA to add new modules to the Makefile/IDEs, I didn't want to do it. But that should be done in the future, so tab navigation can be tested for all widgets.
This commit should be easy to revert if for some reason it's decided it doesn't make sense for 1.3.4. Seems to me it's a good idea to get features like this in early, as our release schedule lately has been few and far between.. | |
|
#17 | greg.ercolano 13:19 Jul 19, 2016 |
| Also applied to the 1.3-porting branch as r11820. | |
[ Return to Bugs & Features ]
|
| |