-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat gui/notes
#1297
base: master
Are you sure you want to change the base?
Feat gui/notes
#1297
Conversation
The multiple peer elements that can be focused makes this UI trickier than most. Here are my thoughts: The Search box makes sense to me -- hit the hotkey, focus the field -- but none of the other fields are presented that way. I think they should be. Name and comments could be styled similarly to the Search field, with their own hotkeys. Presumably this will obviate the need for the "Edit" hotkey at the bottom -- I didn't look at the code for that, but from the UI I'm not actually sure what that does. I am concerned about TextEditor's intrusion into EditField's use case. TextEditor is the more featureful widget, but I'd rather see EditField turned into a subclass of TextEditor than TextEditor in "single line" mode just being offered as an alternative to EditField with slightly different semantics and behavior. I can understand why you like the style presentation you have, with the edit areas surrounded by a frame, and I'm not opposed to it. However, I don't like how this is bifurcating the single line editor UX across DFHack tools. The Notes list is an additional wrinkle. A hotkey to focus the list can work, or you can assign hotkeys for navigating the list regardless of what other field has the focus. This is what |
I agree and I am happy to follow this way if you decide we can do it. The EditField current functionality is very limited and it would be much more useful if could support most of the TextEditor features. |
I agree, going to change it. |
So, the Why like this? I think its not a problem - IMO typically user will edit notes by clicking them on map. |
773e21f
to
5ff4e50
Compare
Yeah, I'm definitely not suggesting any changes about that for this PR. Once TextEditor is stable enough to move to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes from play testing:
- the first two characters of a search don't have an effect. You can search for
ZZ
and it will match all entries, regardless of whether they have anyz
s in them - Alt-s for search has precedent, but "action" hotkeys should be Ctrl hotkeys
- I think the player should be returned to
gui/notes
after placing a new note, similar to the flow for editing an existing note - I think the displayed note name/comment text should follow the list highlight -- that it, it should be updated on
on_select
, noton_submit
.on_submit
can still control the zoom - I keep expecting the text on the right to be editable in-situ and not require a new window to come up, but I realize having so many focusable elements on one page might be impractical. @Ozzatron what do you think here?
gui/notes.lua
Outdated
-- allow cursor up/down to be used to navigate the notes list | ||
if keys.KEYBOARD_CURSOR_UP or keys.KEYBOARD_CURSOR_DOWN then | ||
return false | ||
end | ||
|
||
return NotesSearchField.super.onInput(self, keys) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense for TextEditor to not handle KEYBOARD_CURSOR_UP
and KEYBOARD_CURSOR_DOWN
when one_line_mode=true
? Then this function would not be needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
row_height=1, | ||
on_submit=function (ind, note) | ||
self:loadNote(note) | ||
dfhack.gui.pauseRecenter(note.point.pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be better to use dfhack.gui.revealInDwarfmodeMap(note.point.pos, true, true)
(or (note.point.pos, false, true)
) so the target square gets a flashing highlight. This would help identify the specific map marker if there are many in the area.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the flashlight do not work if we render something custom on the map - and we do, the note marker.
any idea how it can be fixed (if it should be fixed)?
Please consider already have a need for the dedicated modal on edit to react on user clicks on the map notes. |
a37ebe1
to
5742e4e
Compare
Changes:
gui/notes
tool that allow to search, add, update and delete notesHome
/End
shortcut to control line cursor to TextEditorDelete
shortuct to control line cursor to TextEditor