feature: enhance calendar widget with time picker #650

seflue · 2024-01-14T00:00:38Z

When creating a date using the calendar widget, a time can now set directly from the widget.

t jumps to the time picker
h/l switches between hour, "big" minute and "small" minute selection
j/k decreases/increases the time

The step size for "big" and "small" minutes can be configured. Default is 15 and 5 minutes.
When adjusting hours, the minutes are rounded or not (this behavior can be disabled in the config, if too confusing - I find it quite useful).

See also #607.

kristijanhusak

Thanks for the PR! This looks good, but I would like to slightly change things around. My first comment sums up the biggest changes.

We could also refactor the render function and split it into chunks that can be reused, like render_days, render_time, render_help. That way we can easily rerender only a section of it if necessary.

kristijanhusak · 2024-01-15T15:16:16Z

lua/orgmode/config/defaults.lua

@@ -35,6 +35,8 @@ local DefaultConfig = {
  org_highlight_latex_and_related = nil,
  org_custom_exports = {},
  org_indent_mode = 'indent',
+  org_time_picker_min_big = 15,


Let's remove these options and rely only on org_time_stamp_rounding_minutes. Holding j/k updates the time fast enough, so we can stick with only one step.

Regarding the rounding, I would suggest going with additional mappings instead of having it as an option. There is a similar feature to be implemented for regular time up/down (#396) and Emacs uses a mapping with different prefix.

We can either go with uppercase J/K or maybe <leader>j/k.

I actually really would like to keep it, because I like it in the daily usage. My argument is precision. I often schedule meetings to a quarter of an hour, holding the 5 min key is not the same. If different preferences exist, I rather would make it optional via configuration than passing on it completely.
Regarding the rounding I played with different options and came up with the solution which felt most fluent to me. But because I'm not sure, how it feels to other people, I made it optional, because it could feel surprising to others.

Ok, lets just create a separate calendar config option and nest all these settings inside. I want to separate non Orgmode configuration.

I wrapped all calendar related settings into a calendar table as you suggested.

lua/orgmode/objects/calendar.lua

lua/orgmode/objects/date.lua

lua/orgmode/objects/calendar.lua

To make it more convenient to scheduled dates with particular times extends the existing date picker with a time picker. - t adds a time and jumps to the time picker - j and k decrease or increase the time, counts are supported - h and l determine if the time is shifted about an hour, 10 minutes or 5 minutes - T clears the time - Esc leaves the time picker first and pressed secondly the date picker

When entering the date via direct input, provide the currently selected date as default value.

When adjusting an odd minute value, we first want to jump to the next flat 10 or 5 minute value.

seflue · 2024-05-28T07:37:32Z

lua/orgmode/config/defaults.lua

+    round_min_with_hours = true,
+    min_big_step = 15,
+    min_small_step = 5,
+    start_from_sunday = false,


I changed this to a boolean and gave it a clearer name.

seflue · 2024-05-28T08:14:13Z

Somehow, I got a regression with the date mapping tests, need to investigate that later.

seflue · 2024-05-28T08:30:43Z

lua/orgmode/objects/calendar.lua

@@ -150,10 +150,9 @@ function Calendar:render()
  vim.api.nvim_set_option_value('modifiable', true, { buf = self.buf })

  local cal_rows = { {}, {}, {}, {}, {}, {} } -- the calendar rows
-  local start_from_sunday = config.calendar_week_start_day == 0


@kristijanhusak It seems, that this changes to the config introduced a regression to how the date mapping tests work. But I currently don't see why ...

kristijanhusak

@seflue lets keep the calendar week start variable and adapt the rest according to it. That might fix the issue with the tests.

DOCS.md

seflue · 2024-05-28T14:18:17Z

@seflue lets keep the calendar week start variable and adapt the rest according to it. That might fix the issue with the tests.

Everything is fixed now and tests are green. Are we ready to merge, finally? 😅

kristijanhusak

Are we ready to merge, finally?

We're almost there, but a few more things needs to be addressed, sorry. I'll write them down, and put a video below showing all of them in the same order

Add an underline to a currently selected day only when switching to a time picker. Currently, it highlights on every movement, but it's not consistent if user is not using bound movement keys (I'm using w and b in the video)
Triggering any other mappings while in the time picker mode moves the cursor but still manipulates the date. We should probably go out of the time picker mode once any of those are pressed
Shortcut shows [t] Enter time option when we enter the time mode until we change the time, then it switches.

Video:

calendar-time.mp4

I'm aware you spent a lot of time on this implementation, but what do you think about switching it up a bit to be more time centric when in time mode? Let me give an example:

Currently with this PR, we show date and time like this:

             June 2024
 Mon  Tue  Wed  Thu  Fri  Sat  Sun
                          01   02 
 03   04   05   06   07   08   09 
 10   11   12   13   14   15   16 
 17   18   19   20   21   22   23 
 24   25   26   27   28   29   30 
  
               16:30              

 [<] - prev month  [>] - next month
 [.] - today   [Enter] - select day
 [i] - enter date
 [t] - enter time

What do you think about switching the whole UI when you go to a time picker mode? Mappings would work the similar, but you wouldn't have to worry about rendering the calendar view and all of the other things. Basically, we could switch the calendar to have 2 views, and code them separately inside the calendar.

Time view would be something like this (wrote it manually just to get an idea):

    Date: 2024-05-28

           16:30

 [Enter] - select time
 [T] - clear time
 [Q] - Go back

This view should not be able to quit the calendar, so it would be more of a sub-view.
It's not flexible as the current solution, but it should be more maintainable and (somewhat) easier to code.

lua/orgmode/objects/calendar.lua

Because it's a matter of preference, in which steps the user want's to jump through minutes, we make it configurable.

To be able to clear times if no timestamp was set when the picker was loaded, we need to set the clear-time keymapping during rerendering of time.

- wrap everything into calendar - cleanup names - use boolean "start_from_sunday"

seflue · 2024-05-28T18:03:08Z

Are we ready to merge, finally?

We're almost there, but a few more things needs to be addressed, sorry. I'll write them down, and put a video below showing all of them in the same order

Add an underline to a currently selected day only when switching to a time picker. Currently, it highlights on every movement, but it's not consistent if user is not using bound movement keys (I'm using w and b in the video)

This was my initial solution, but because I always adjust the date with hjkl, I actually like, that it updates on every keypress. I am aware of the glitch, that it doesn't update with w and b, which is hard to fix with the current structural design of the calendar.

Triggering any other mappings while in the time picker mode moves the cursor but still manipulates the date. We should probably go out of the time picker mode once any of those are pressed

I am aware of this glitch. To fix it, it would be much easier with a complete overhaul of the whole code design of the calender widget.

Shortcut shows [t] Enter time option when we enter the time mode until we change the time, then it switches.

Video:

calendar-time.mp4

I just fixed this, it behaves now consistently.

I'm aware you spent a lot of time on this implementation, but what do you think about switching it up a bit to be more time centric when in time mode? Let me give an example:

Currently with this PR, we show date and time like this:
             June 2024
 Mon  Tue  Wed  Thu  Fri  Sat  Sun
                          01   02 
 03   04   05   06   07   08   09 
 10   11   12   13   14   15   16 
 17   18   19   20   21   22   23 
 24   25   26   27   28   29   30 
  
               16:30              

 [<] - prev month  [>] - next month
 [.] - today   [Enter] - select day
 [i] - enter date
 [t] - enter time
What do you think about switching the whole UI when you go to a time picker mode? Mappings would work the similar, but you wouldn't have to worry about rendering the calendar view and all of the other things. Basically, we could switch the calendar to have 2 views, and code them separately inside the calendar.

Time view would be something like this (wrote it manually just to get an idea):
    Date: 2024-05-28

           16:30

 [Enter] - select time
 [T] - clear time
 [Q] - Go back
This view should not be able to quit the calendar, so it would be more of a sub-view. It's not flexible as the current solution, but it should be more maintainable and (somewhat) easier to code.

This would be an overhaul but to be honest, I like to see the week structure of a month while planning the time because I typically think in week days and having not to memorize, which weekday a particular day is, safes me a couple of brain-cycles, while working with the widget - which is part of the fun using it.

All in all, I would very appreciate, if we can merge the PR for now like it is. Perfect is the enemy of the good and the feature already provides a lot of value - and no upstream user is benefitting from it for nearly half a year.

My main motivation to implement this was to make my own day-to-day work with timestamps easier and more fun and I already achieved this with the very first implementation.

Now I am a bit tired of rebasing this branch all the time to be able to use my feature and at the same time keeping up with the upstream changes, while worrying that another breaking change will cause more time consuming merge conflicts.

Honestly it was quite some work to update my code after your refactoring of the Calendar class, which was structurally not a very complicated change, but because it touched nearly every 4th line it conflicted with all my commits. Actually, it would have saved us both some time, if we would have merged the initial solution and improved it with further PRs.

Let's bring what we have to the users, gather some feedback and then think on improvements. I already have some ideas, how the class can be much better implemented, but for now I want to use the feature without daily maintenance of rebasing.

seflue · 2024-05-28T18:16:44Z

One last argument for not waiting longer with merging: While using nvim-orgmode on a daily basis I am discovering a lot of much rougher edges regarding usability, which I would actually like to spend time on to fix them. But this long-running branch is already dragging a bit on my motivation, because it feels like a never-ending story.

kristijanhusak

Ok, sorry for bothering you so much. I'll address some of the smaller things.
Thanks!

* feat: implement time picker To make it more convenient to scheduled dates with particular times extends the existing date picker with a time picker. - t adds a time and jumps to the time picker - j and k decrease or increase the time, counts are supported - h and l determine if the time is shifted about an hour, 10 minutes or 5 minutes - T clears the time - Esc leaves the time picker first and pressed secondly the date picker * feat(date): add default to 'Enter date' When entering the date via direct input, provide the currently selected date as default value. * chore: fix warnings in calendar * fix: jump to descrete 10 or 5 minute steps When adjusting an odd minute value, we first want to jump to the next flat 10 or 5 minute value. * feat: make minute steps configurable Because it's a matter of preference, in which steps the user want's to jump through minutes, we make it configurable. * feat: highlight currently selected date * refactor: extract date highlighting into function * feat: update date selection regularly * feat: round minutes on adjusting hours * fix: show time picker also when changing date * fix: enable clear time keymap consistently To be able to clear times if no timestamp was set when the picker was loaded, we need to set the clear-time keymapping during rerendering of time. * refactor: integrate highlights into new function * fix: allow to switch back to day selection * feat: overhaul config options - wrap everything into calendar - cleanup names - use boolean "start_from_sunday" * doc: add some docs for the config options * doc: fix typo --------- Co-authored-by: Sebastian Flügge <[email protected]>

kristijanhusak reviewed Jan 15, 2024

View reviewed changes

kristijanhusak force-pushed the master branch from 44754e8 to 5143eed Compare February 16, 2024 18:21

seflue force-pushed the feature/schedule_clock branch from c5900a4 to 23f91ca Compare February 17, 2024 14:18

seflue force-pushed the feature/schedule_clock branch from 23f91ca to 8d7fcfd Compare March 3, 2024 19:07

seflue mentioned this pull request May 25, 2024

Add tests for archiving subtree #734

Merged

seflue force-pushed the feature/schedule_clock branch from 8d7fcfd to 5345e65 Compare May 25, 2024 22:52

seflue mentioned this pull request May 25, 2024

fix(date): persist all attributes passed by options #735

Merged

seflue force-pushed the feature/schedule_clock branch from 5345e65 to be3d95e Compare May 25, 2024 23:11

seflue commented May 25, 2024

View reviewed changes

lua/orgmode/objects/calendar.lua Outdated Show resolved Hide resolved

seflue commented May 25, 2024

View reviewed changes

lua/orgmode/objects/calendar.lua Outdated Show resolved Hide resolved

seflue force-pushed the feature/schedule_clock branch 2 times, most recently from 28f58b1 to e9474bb Compare May 28, 2024 05:50

seflue added 4 commits May 28, 2024 07:51

feat(date): add default to 'Enter date'

6126f78

When entering the date via direct input, provide the currently selected date as default value.

chore: fix warnings in calendar

60963a7

fix: jump to descrete 10 or 5 minute steps

5e0e9ca

When adjusting an odd minute value, we first want to jump to the next flat 10 or 5 minute value.

seflue force-pushed the feature/schedule_clock branch 2 times, most recently from 1ef4e7a to a6e21bf Compare May 28, 2024 06:08

seflue commented May 28, 2024

View reviewed changes

seflue force-pushed the feature/schedule_clock branch from c5b0716 to b4ebc63 Compare May 28, 2024 08:04

seflue commented May 28, 2024

View reviewed changes

kristijanhusak requested changes May 28, 2024

View reviewed changes

DOCS.md Outdated Show resolved Hide resolved

DOCS.md Outdated Show resolved Hide resolved

seflue force-pushed the feature/schedule_clock branch from b4ebc63 to 9c628b5 Compare May 28, 2024 14:11

kristijanhusak requested changes May 28, 2024

View reviewed changes

lua/orgmode/objects/calendar.lua Outdated Show resolved Hide resolved

seflue force-pushed the feature/schedule_clock branch from 9c628b5 to 2dd2ce8 Compare May 28, 2024 16:52

seflue added 3 commits May 28, 2024 18:54

feat: make minute steps configurable

9b74096

Because it's a matter of preference, in which steps the user want's to jump through minutes, we make it configurable.

feat: highlight currently selected date

566b809

refactor: extract date highlighting into function

f522eb4

seflue added 9 commits May 28, 2024 18:54

feat: update date selection regularly

a81d5f6

feat: round minutes on adjusting hours

74b9178

fix: show time picker also when changing date

a52a268

fix: enable clear time keymap consistently

f4da11c

To be able to clear times if no timestamp was set when the picker was loaded, we need to set the clear-time keymapping during rerendering of time.

refactor: integrate highlights into new function

e483f6d

fix: allow to switch back to day selection

9db1d66

feat: overhaul config options

5519583

- wrap everything into calendar - cleanup names - use boolean "start_from_sunday"

doc: add some docs for the config options

eaffdfc

doc: fix typo

6a64028

seflue force-pushed the feature/schedule_clock branch from 2dd2ce8 to 6a64028 Compare May 28, 2024 16:55

kristijanhusak approved these changes May 28, 2024

View reviewed changes

kristijanhusak merged commit 7af6cbd into nvim-orgmode:master May 28, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: enhance calendar widget with time picker #650

feature: enhance calendar widget with time picker #650

seflue commented Jan 14, 2024

kristijanhusak left a comment

kristijanhusak Jan 15, 2024

seflue Jan 16, 2024 •

edited

Loading

kristijanhusak Jan 17, 2024

seflue May 28, 2024

seflue May 28, 2024

seflue commented May 28, 2024

seflue May 28, 2024

kristijanhusak left a comment

seflue commented May 28, 2024

kristijanhusak left a comment

seflue commented May 28, 2024 •

edited

Loading

seflue commented May 28, 2024 •

edited

Loading

kristijanhusak left a comment

feature: enhance calendar widget with time picker #650

feature: enhance calendar widget with time picker #650

Conversation

seflue commented Jan 14, 2024

kristijanhusak left a comment

Choose a reason for hiding this comment

kristijanhusak Jan 15, 2024

Choose a reason for hiding this comment

seflue Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

kristijanhusak Jan 17, 2024

Choose a reason for hiding this comment

seflue May 28, 2024

Choose a reason for hiding this comment

seflue May 28, 2024

Choose a reason for hiding this comment

seflue commented May 28, 2024

seflue May 28, 2024

Choose a reason for hiding this comment

kristijanhusak left a comment

Choose a reason for hiding this comment

seflue commented May 28, 2024

kristijanhusak left a comment

Choose a reason for hiding this comment

seflue commented May 28, 2024 • edited Loading

seflue commented May 28, 2024 • edited Loading

kristijanhusak left a comment

Choose a reason for hiding this comment

seflue Jan 16, 2024 •

edited

Loading

seflue commented May 28, 2024 •

edited

Loading

seflue commented May 28, 2024 •

edited

Loading