feat: add month calendar #3667

TomJGooding · 2023-11-12T21:41:12Z

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

Date Picker #3576

Checklist:

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

darrenburns

Early review with some comments and general out loud thinking. Good progress!

src/textual/widgets/_month_calendar.py

darrenburns · 2023-11-13T17:06:00Z

docs/examples/widgets/month_calendar.py

+from textual.widgets import MonthCalendar
+
+
+class MonthCalendarApp(App):


Adding a CSS variable containing the following CSS to this app yields better results. I reckon we should add the equivalent CSS to the widget itself to override the default DataTable behaviour (unless you can think of a reason not to).

MonthCalendar { height: auto; width: auto; } MonthCalendar > DataTable { height: auto; width: auto; }

We don't do this in the DataTable itself because it's intended to scroll, but this widget has a very constrained height compared to the DataTable which can be arbitrarily large.

Sorry I should have mentioned, I haven't started looking at the DEFAULT_CSS yet as I wanted to focus on the core functionality first. Do you think the table/columns widths should just be static, or allow the developer to specify the column width (or perhaps even grow dynamically according to the widget width)?

I haven't started looking at the DEFAULT_CSS yet as I wanted to focus on the core functionality first.

Seems sensible.

Do you think the table/columns widths should just be static, or allow the developer to specify the column width (or perhaps even grow dynamically according to the widget width)?

I think static for now.

I've now added some basic DEFAULT_CSS in cfcac14. I've flagged min-width as TODO as this might depend on locale?

darrenburns · 2023-11-13T17:07:48Z

src/textual/widgets/_month_calendar.py

+class MonthCalendar(Widget):
+    year: Reactive[int | None] = Reactive[int | None](None)
+    month: Reactive[int | None] = Reactive[int | None](None)
+    first_weekday: Reactive[int] = Reactive(0)


I'm wondering about using Literal["monday", "tuesday", ...] here to remove any second-guessing about how days are indexed. Will leave it as an open thought here if you or anyone else wants to chip in.

Perhaps this could be an IntEnum similar to the calendar module in 3.12?

darrenburns · 2023-11-13T17:11:52Z

src/textual/widgets/_month_calendar.py

+        self.first_weekday = first_weekday
+
+    def compose(self) -> ComposeResult:
+        yield DataTable()


Open thoughts: I wonder if we should display the month name above the DataTable. I wonder if it should be something that can be toggled.

I'm assuming this would simply display "November 2023" for example, and then if developers wanted something different they would be expected to create a custom widget and set MonthCalendar.month_header to False?

I'm not sure - I guess the month name would be defined by locale too? @willmcgugan any thoughts?

TomJGooding · 2023-11-14T14:17:21Z

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

Look into options for handling reactive-related methods firing before the DOM is mounted #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 · 2023-11-20T12:20:18Z

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 · 2023-11-20T13:22:53Z

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 · 2023-11-20T13:55:48Z

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

TomJGooding · 2023-11-29T14:31:46Z

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!)

…nth-calendar

willmcgugan · 2024-08-12T12:55:42Z

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 · 2024-08-12T13:12:02Z

Any thoughts on the component class thing @willmcgugan?

willmcgugan · 2024-08-12T13:43:14Z

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

darrenburns · 2024-08-12T13:58:18Z

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 · 2024-08-13T12:56:48Z

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.

darrenburns · 2024-09-10T12:57:46Z

Just to give an example of the issue:

class CalendarGrid(DataTable, inherit_bindings=False):
    # TODO: Ideally we want to hide that there's a DataTable underneath the
    # `MonthCalendar` widget. Is there any mechanism so component classes could be
    # defined in the parent and substitute the component styles in the child?
    # For example, allow styling the header using `.month-calendar--header`
    # rather than `.datatable--header`?
    DEFAULT_CSS = """
    CalendarGrid {
        height: auto;
        width: auto;
        .datatable--header {
            background: $surface;
        }
    }
    """

We need to use the .datatable--header component class selector, which is a bit ugly!

willmcgugan · 2024-09-11T12:57:54Z

We need to use the .datatable--header component class selector, which is a bit ugly!

It is a bit of a wart. We discussed it in the office and didn't come up with a conclusion. Suggest we go with this for now.

willmcgugan

Hi Tom, sorry for the delay again. Getting caught up.

I know this is marked as draft, just wanted to add a few comments to avoid blocking while waiting for my input.

willmcgugan · 2024-09-11T13:02:44Z

src/textual/widgets/_month_calendar.py

+
+from rich.text import Text
+
+from textual import on


Could you change these to relative imports, which is the convention we use elsewhere.

I think we are going to switch to absolute imports eventually, but best to be consistent for now...

I've reverted back to absolute imports following #4997!

willmcgugan · 2024-09-11T13:05:07Z

src/textual/widgets/_month_calendar.py

+        "month-calendar--outside-month",
+    }
+
+    date: Reactive[datetime.date] = Reactive(datetime.date.today())


The today() is evaluated at import time. We probably want it to be evaluated when the widget is instantiated.

Easy fix: reactives take a callable. So we can just drop the ().

Fixed in 172fbbe.

willmcgugan · 2024-09-11T13:05:18Z

src/textual/widgets/_month_calendar.py

+    show_cursor: Reactive[bool] = Reactive(True)
+    show_other_months: Reactive[bool] = Reactive(True)
+
+    class DateHighlighted(Message):


Docstring pls

willmcgugan · 2024-09-11T13:05:27Z

src/textual/widgets/_month_calendar.py

+        def control(self) -> MonthCalendar:
+            return self.month_calendar
+
+    class DateSelected(Message):


Docstring pls

willmcgugan · 2024-09-11T13:06:07Z

src/textual/widgets/_month_calendar.py

+        event.stop()
+        pass
+
+    def previous_month(self) -> None:


Docstrings on all methods pls. With the exception of inherited methods / handlers.

Sorry just to confirm, do all watch and action methods need docstings? Also should these methods be private (i.e. include a leading underscore)? I did have a look at other widgets for guidance but there seems to be a mix.

Actions, yes. Just a note to say what they do. Watch methods, the docstrings seems rendundant, so we often skip those unless you want to document for future devs why they are needed.

With regards to marking them private, if there is no earthly reason the user would want to override those, then its best to make them private...

Thanks, I've added those docstrings but wasn't sure if they should be private so left as-is for now.

This reverts commit 694a24d.

TomJGooding added 2 commits November 12, 2023 21:06

feat: add month calendar

38e3fb6

impoort future annotations

9c9d4c2

darrenburns reviewed Nov 13, 2023

View reviewed changes

TomJGooding added 2 commits November 14, 2023 08:31

update reactives and remove compute method

d4fb00c

remove unused var import

acdfc87

TomJGooding added 7 commits November 14, 2023 18:42

add provisional watch methods for year and month

073c3cd

add show cursor

ab1aa6d

add previous and next year methods

684b2be

add previous and next month methods

c0d44e8

add method to get date coordinate

5cc2b85

add move cursor method

055d788

experiment with adding pauses to tests for ci

5bd8bc2

TomJGooding added 11 commits November 20, 2023 21:01

Merge branch 'main' into feat-add-month-calendar

7939c54

precommit isort

53b1ae6

remove old pauses in tests

7a7266e

add is_current_month property

56b380a

watch reactive first weekday change

4e139c9

add default cursor date

d6cec28

update cursor date on cell highlighted

42de0f9

add bindings to example

726274b

add basic default css

cfcac14

Merge branch 'main' into feat-add-month-calendar

caea6eb

update cursor date with relative delta

334e771

Merge branch 'main' of github.com:Textualize/textual into feat-add-mo…

5a5eed2

…nth-calendar

darrenburns requested a review from willmcgugan December 12, 2023 09:48

TomJGooding added 6 commits August 13, 2024 11:21

replace relativedelta with timedelta for days and weeks

3f7d56f

add tests that prev/next month methods account for leap years

e72df0b

replace relativedelta in prev/next year methods

6fa86ac

add tests that prev/next month methods account for leap/non-leap years

328bf84

replace relativedelta in prev/next month methods

d44e3e5

replace remaining relativedelta instances and remove import

077bd63

TomJGooding added 4 commits August 13, 2024 15:52

tidy tests to help readability

a7d13ab

refactor _get_calendar_dates method

530adec

rename calendar table to grid

aec97b0

Merge branch 'main' into feat-add-month-calendar

6789b01

willmcgugan reviewed Sep 11, 2024

View reviewed changes

TomJGooding added 12 commits September 11, 2024 17:30

Merge branch 'main' into feat-add-month-calendar

c6efdde

change to relative imports

694a24d

fix today by dropping the ()

172fbbe

add class docstrings

c9b7bd0

Revert "change to relative imports"

ed0f127

This reverts commit 694a24d.

add docstrings to messages

e59d91c

add docstrings for methods

69a1b2a

add docstrings for action methods

52e2f38

Merge branch 'main' into feat-add-month-calendar

9a92dee

fix AttributeError by restoring order in init

de46d7a

Merge branch 'main' into feat-add-month-calendar

5c9c2be

update casing for binding descriptions

a6c38bb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add month calendar #3667

feat: add month calendar #3667

TomJGooding commented Nov 12, 2023 •

edited

Loading

darrenburns left a comment

darrenburns Nov 13, 2023

TomJGooding Nov 14, 2023 •

edited

Loading

darrenburns Nov 20, 2023

TomJGooding Nov 23, 2023

darrenburns Nov 13, 2023

TomJGooding Nov 23, 2023 •

edited

Loading

darrenburns Nov 13, 2023

TomJGooding Nov 23, 2023

darrenburns Dec 12, 2023

TomJGooding commented Nov 14, 2023 •

edited

Loading

darrenburns commented Nov 20, 2023

TomJGooding commented Nov 20, 2023

darrenburns commented Nov 20, 2023

TomJGooding commented Nov 29, 2023 •

edited

Loading

willmcgugan commented Aug 12, 2024

darrenburns commented Aug 12, 2024

willmcgugan commented Aug 12, 2024

darrenburns commented Aug 12, 2024 •

edited

Loading

TomJGooding commented Aug 13, 2024

darrenburns commented Sep 10, 2024

willmcgugan commented Sep 11, 2024

willmcgugan left a comment

willmcgugan Sep 11, 2024

TomJGooding Sep 13, 2024

willmcgugan Sep 11, 2024

TomJGooding Sep 13, 2024

willmcgugan Sep 11, 2024

willmcgugan Sep 11, 2024

willmcgugan Sep 11, 2024

TomJGooding Sep 14, 2024

willmcgugan Sep 14, 2024

TomJGooding Sep 14, 2024

		from textual.widgets import MonthCalendar


		class MonthCalendarApp(App):

feat: add month calendar #3667

Are you sure you want to change the base?

feat: add month calendar #3667

Conversation

TomJGooding commented Nov 12, 2023 • edited Loading

Description

Related Issue

Checklist:

darrenburns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomJGooding Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomJGooding Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomJGooding commented Nov 14, 2023 • edited Loading

darrenburns commented Nov 20, 2023

TomJGooding commented Nov 20, 2023

darrenburns commented Nov 20, 2023

TomJGooding commented Nov 29, 2023 • edited Loading

willmcgugan commented Aug 12, 2024

darrenburns commented Aug 12, 2024

willmcgugan commented Aug 12, 2024

darrenburns commented Aug 12, 2024 • edited Loading

TomJGooding commented Aug 13, 2024

darrenburns commented Sep 10, 2024

willmcgugan commented Sep 11, 2024

willmcgugan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomJGooding commented Nov 12, 2023 •

edited

Loading

TomJGooding Nov 14, 2023 •

edited

Loading

TomJGooding Nov 23, 2023 •

edited

Loading

TomJGooding commented Nov 14, 2023 •

edited

Loading

TomJGooding commented Nov 29, 2023 •

edited

Loading

darrenburns commented Aug 12, 2024 •

edited

Loading