feat: add month calendar

[TomJGooding]

Description

Add a MonthCalendar widget, intended to form part of a DatePicker widget.

[!NOTE] Currently very much a work in progress wanting feedback from Textual maintainers and users!

Related Issue

• #3576

Checklist:

• [ ] Docstrings on all new or modified functions / classes
• [ ] Updated documentation
• [ ] Updated CHANGELOG.md (where appropriate)

[TomJGooding]

I'm struggling with handling the reactives for this widget, due to a class of problem explained this issue:

• #3011

I'm afraid I'm blocked on this, unless there's a workaround that would include a cursor reactive. My current call_after_refresh is really a hacky workaround just so tests pass locally (though obviously not in CI!)

[darrenburns]

Hi @TomJGooding, could you try adding a boolean @property called is_mounted to Widget which checks that self.mounted_event.is_set(), and then inside your watchers, only do things relating to the DOM (querying, updating, etc.) if the DOM is ready? Would that resolve the issue?

Let me know if I've misunderstood the issue!

[TomJGooding]

Thanks Darren for your help, it looks like this works:

    def watch_month(self) -> None:
        if not self.is_mounted:
            return
        self._update_calendar_days()

Would it be better to add this new Widget.is_mounted property in a separate PR (linked to the issue #3011 mentioned above)?

[darrenburns]

No problem! I think separate PR would be great for that.

[TomJGooding]

My latest commit (334e771) will update the calendar cursor to the same highlighted day when the month changes.

Here's a quick demo recording, notice this accounts for when the new month doesn't have 31 days:

This requires the python-dateutil package using relativedelta to add/subtract months. I was able to import this, but I'm not sure if only because it is required by one of the dev dependencies?

The problem with updating the cursor when the month changes is I'm not sure how to handle when the highlighted date is outside the month. For example in the demo recording, on the initial month the cursor could be highlighting the first Monday (31st). Where should the cursor highlight when the month changes? If changed to just the previous month, it probably should just highlight the same date, but what if the updated calendar is months or years different?

Perhaps if you don't mind (I appreciate how busy you are), this would a good time for a pre-review review? Tagging @darrenburns and @davep but any feedback would be welcome! Thanks again for your help so far.

(Also just to note, the failing test is a keyline snapshot mismatch and not related to this PR!)

[darrenburns]

@willmcgugan there are a few outstanding API questions here - could you have a look?

[darrenburns]

Sorry for the delay Tom, I just got back from holiday yesterday!

Some thoughts after playing with the example:

• If we move the cursor to one of the greyed-out days (from the previous or next month), should that entire month be brought into view? It does in Chrome and it feels quite nice, but I'm not sure if it only feels understandable because of the animation.
• Thinking about how end users would interact with this, I wonder if having a single date reactive attribute might be better? We could potentially collapse year, month and cursor_date into a single reactive date which, when set, updates the cursor position and brings the month into view. Or is there some other benefit to keeping them separate that I'm not seeing yet?

Edit: Also, just a note regarding the snapshot test failing - we had a temporary breakage a while back merged into main that was quite quickly fixed - it seems you last merged main at an unfortunate time 😄

[willmcgugan]

Yeah, sorry for the delay Tom. I think you are on the right track. I'll let Darren et al work with you here.

There are a lot of quite fiddly user interactions with a datepicker. I suspect the best way forward is to attempt it and see how it feels. Browsers all do this quite differently, but we can use them as a guide.

[TomJGooding]

I think the design for this MonthCalendar is finding the right balance between it ultimately being intended for the DatePicker but also providing a general purpose calendar display.

I wonder if having a single date reactive attribute might be better?

I did wonder this too, but wasn't sure if setting a date for a calendar without the cursor might seem odd?

If we move the cursor to one of the greyed-out days (from the previous or next month), should that entire month be brought into view?

I think this makes sense for the DatePicker, but perhaps in other contexts you wouldn't want the calendar month to change like this.

EDIT: Sorry for the noise from previous edits, it has been a little while since I last looked at this! I've realised that 1) having a single date reactive would only work if 2) the month was changed if a greyed-out day was highlighted.

[TomJGooding]

I've now changed the MonthCalendar to have a single date reactive and hopefully found a solution for the problem mentioned above.

If show_other_months is True (the default), highlighting a greyed-out date from the previous or next month will update the calendar to bring that entire month into view:

Whereas if show_other_months is False, only dates from the current month will be displayed and other blank cells should not be selectable. The example below shows trying to navigate to cells outside the month:

Currently my method to prevent highlighting these blank cells is perhaps a bit hacky, but I've been trying to avoid a custom DataTable with overrides some methods. But perhaps this is unavoidable as currently you can still hover these cells with the mouse which isn't ideal.

Any feedback when you get chance would be much appreciated!

[TomJGooding]

I'm also now wondering if the calendar should always display six weeks (if this new show_other_months attribute is True).

For example, February 2021 would display like this:

Rather than this like it does currently:

The table changing size when the month is changed is perhaps a bit jarring. This also seems to be how other datepickers in the browser are designed.

What do you think?

[darrenburns]

I like having the greyed out days at the top and bottom. Not only does it fix the sizing issue, but also the empty cell hovering issue, and it'll make navigation easier if you can move through the months using the arrow keys (you'll be able to see where you'll land).

[TomJGooding]

Thanks Darren for the feedback. Hopefully shouldn't be too difficult to change.

Not only does it fix the sizing issue, but also the empty cell hovering issue,

Just to clarify, the problem with the 'blank' cell hovering is only if show_other_months is set to False, so I'd still need to find a solution for that.

[TomJGooding]

After some mashing of the arrow keys, there's definitely some issues with my current implementation of the calendar as it will start to freeze/lag and CPU usage climbs. Initially I thought just caching the calendar_dates matrix was the solution (which probably is a good idea anyway!), but there must be other expensive operations I've missed. Something else for me to look into, really only posted this as a reminder for myself.

[TomJGooding]

[darrenburns]

Looks like great progress :) did you manage to resolve the performance issues you mentioned previously or would you like some help in that area?

[TomJGooding]

Unfortunately not. If you try mashing the arrow keys for a few seconds on the month_calendar.py example app, it will start to lag/freeze, possibly just due to an overload of messages but honestly I'm not really sure.

[willmcgugan]

Hi @TomJGooding Just had a quick look. I can't reproduce any performance problems. Seems to remain responsive for me. Are you on Windows?

[TomJGooding]

Linux. You really do have to be quickly mashing the arrow keys!

[TomJGooding]

I suspect the problem is that the CellHighlighted event will set the reactive date:

https://github.com/Textualize/textual/blob/ca8269173adb97efa7cde3761b19283e2c525c9b/src/textual/widgets/_month_calendar.py#L108-L111

But the watch_date method may then set the table cursor_coordinate:

https://github.com/Textualize/textual/blob/ca8269173adb97efa7cde3761b19283e2c525c9b/src/textual/widgets/_month_calendar.py#L276-L279

If you're really mashing the arrow keys, this will probably create a race condition which causes these performance issues?

I'm afraid I will need some help to resolve this, but obviously appreciate how busy you are at the moment.

[TomJGooding]

I'm not sure if Textual currently has appetite for this new widget, but I thought I would post an update anyway after tinkering with this again over the past few weeks.

You can take the current MonthCalendar widget for a test spin by running the app in docs/examples/widgets/month_calendar.py on this branch.

I think I managed to resolve the performance issues mentioned above (see commit f4549ca), but please let me know if there is a better way of handling this possible race condition.

The key bindings now more closely mirror the behaviour of the date picker in the web browser:

• left/right when the cursor is on the first/last column should wrap around the cursor to previous/next date
• up/down when the cursor is on the first/last row should update the calendar to the previous/next week
• pageup/pagedown should update the calendar to the previous/next month
• ctrl+pageup/pagedown should update the calendar to the previous/next year

I'm still not quite sure how to handle all of this if show_other_months is False, besides other improvements that I am sure that could be made.

I appreciate there's a lot else going on in Textual at the moment, so not expecting a review anytime soon!

[darrenburns]

I've took this for a spin as an end user and the API and it feels good. Only thing I'm feeling a little unsure of is show_other_months (see earlier comment https://github.com/Textualize/textual/pull/3667#discussion_r1674292899).

(Also - sorry for the delay in reviewing, Tom)

[TomJGooding]

@darrenburns No problem, thanks for taking another look at this!

I remember seeing the option to show/hide dates outside the current month in other datepicker/calendar implementations, so at the time this seemed worth adding. But I think the confusion is that many datepickers don't have keyboard navigation, and currently my show_other_months is conflating styling options with limitation options for the date/month.

I probably need to spend some time reviewing this myself again as it has been a few months!

EDIT: I've now changed show_other_months so this is only for styling - what do you think?

I'm also now wondering if show_cursor should actually be read_only and also disable the other bindings to change the month/year etc? Or maybe instead this widget should grow min/max_date options to limit the dates/months that can be navigated/selected?

[darrenburns]

I think I prefer show_other_months being for styling only.

Agreed on the comment about hiding the fact that this extends from DataTable in the component classes, but there's no mechanism for doing that.

Probably needs @willmcgugan input at this point.

[TomJGooding]

Sorry @willmcgugan, I know you have a backlog but I would really appreciate your input.

I'm happy to look at adding some mechanism for the component classes (see 631cde7), but I would need some guidance about where to start.

[willmcgugan]

It looks good! And perfectly functionalal AFAICT.

I'd would be good to avoid the dependency on python-dateutil if possible. Not sure where it is coming from. It may be a dev dependency, which means it would need to be added to the regular dependancies.

It looks like there is only one function from that packet being used. Can we extract that or roll our own maybe?

[darrenburns]

Any thoughts on the component class thing @willmcgugan?

[willmcgugan]

I don't follow the component class thing. Can somebody summarize?

[darrenburns]

To style this e.g. the cursor of this widget, since it inherits from DataTable, developers need to target component classes named after the DataTable, which isn't very nice. ~Tom has added a workaround.~

[TomJGooding]

Thanks @willmcgugan for your feedback. I've now removed the dateutil.relativedelta import which actually wasn't as difficult as I feared!

Hopefully the component classes issue makes sense, but just to clarify I haven't managed to find any workaround. I'm happy to look at adding some mechanism for this, but I'm afraid I would need a little guidance on where to start.