-
Notifications
You must be signed in to change notification settings - Fork 164
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
Add search syntax cheatsheet help popup #725
base: master
Are you sure you want to change the base?
Conversation
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.
Idea is OK for me, seems to work but I have some things to complain about.
Also, It seems a bit inconsistent for me with word
but later [date]
is being used later.
And what the exact difference between word
and "text"
isn't very clear (maybe add spaces in the example?). Also, what about word word
?
GTG/gtk/data/main_window.ui
Outdated
@@ -1,59 +1,61 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<!-- Generated with glade 3.36.0 --> | |||
<!-- Generated with glade 3.38.2 --> |
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.
Possibly "remove" glade introduced automatic changes so reviewing is a bit easier (to see what has been changed). I think git checkout -p GTG/gtk/data/main_window.ui
or something like that allows you to "revert" some changes in a file.
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.
Removed a bit, but it's hard to tell what can be removed from the auto changes without breaking things
I copy/pasted from the table in help, but I agree we could make it clearer |
5b17457
to
ea18452
Compare
Looks better now, but what about |
My first reaction was, "Don't we have GtkTooltips for this?" but I understand that those are probably too limited in styling (especially text wrapping etc.) and that the popover approach is preferrable. Just gotta make sure it doesn't interfere with focus or keyboard use in general or something like that (I don't have specific in mind here, I don't know why I'm saying this vague theoretical concern, but I'm saying it just in case there is something we haven't thought of). I think the UI you are proposing looks pretty neat. Is it supposed to support the As for the quickadd entry, is there much more to document than what's already in the tooltip? If not, then it could probably remain simply as a one-line tooltip like it is now... |
Another theoretical concern (but probably not an actual issue): are the translated versions of the soon, now, etc. keywords accepted as input in the app in practice? I think so, but I don't remember if it worked 100%. In any case, at first glance and very broadly looking (and without testing), it looks good. What could possibly go wrong? :) |
At the moment this is just for the search box and you manually need to click on the little icon in the search entry (not sure if it is keyboard friendly). And to my knowledge, the search box and quick add entries accept translated keywords. |
Oh right this is for the search box at the top, which doesn't even have a tooltip right now, Makes a lot of sense, now that I'm looking at this again, given how many search operators there are. @diegogangl would need to rebase this to work with |
Quick idea I had, I can never quite remember this. The button in the search field toggles the popup with a quick cheatsheet of the syntax.
We could do the same with the quick add entry. Replacing the emojis button with one of this.