Skip to content

refactor: partNameMap as a Lit directive#1673

Merged
damyanpetev merged 11 commits intomasterfrom
rkaraivanov/partMap-directive
May 29, 2025
Merged

refactor: partNameMap as a Lit directive#1673
damyanpetev merged 11 commits intomasterfrom
rkaraivanov/partMap-directive

Conversation

@rkaraivanov
Copy link
Copy Markdown
Member

No description provided.

@rkaraivanov rkaraivanov requested a review from damyanpetev May 22, 2025 13:24
rkaraivanov and others added 4 commits May 28, 2025 15:56
Comment thread src/components/calendar/days-view/days-view.ts
Comment thread src/components/calendar/days-view/days-view.ts
Comment on lines +132 to +133
aria-selected=${this.selected}
aria-disabled=${this.disabled}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: We should probably set pattern for bool attrs. Wouldn't these be better off with the ? prefix to skip DOM false values? Applies across the board really.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really because aria-selected and the backing property are typed as string | null, so Lit kind of complains about the ? bindings there.
I do agree we should come with some mechanism that is not the verbose ifDefined(..blah blah) juggle that we currently do at some parts of the codebase.

@rkaraivanov rkaraivanov requested a review from damyanpetev May 29, 2025 11:43
@damyanpetev damyanpetev merged commit f8740f2 into master May 29, 2025
5 checks passed
@damyanpetev damyanpetev deleted the rkaraivanov/partMap-directive branch May 29, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants