Skip to content
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

fix(ui5-calendar): add tooltips to special dates #10335

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

unazko
Copy link
Contributor

@unazko unazko commented Dec 7, 2024

Fixes: #10334

@unazko unazko requested a review from Todor-ads December 9, 2024 08:20
Copy link
Contributor

@tsanislavgatev tsanislavgatev left a comment

Choose a reason for hiding this comment

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

We need to remove the connection between CalendarLegend and DayPicker, currently they are tightly coupled and it's in contradiction with least knowledge principle(Law of Demeter). I would suggest removing the connection between them and move communication on Calendar level. We already pass the specialDates from Calendar to DayPicker, so we can now just use the same objects, add a private _tooltip property and have the tooltip(text property) from CalendarLegendItems. We can use attachInvalidate to listen for CalendarLegend invalidation and react upon, because apps can change the text or add more CalendarLegendItems and since the legend invalidates on child change, it will notify the Calendar as well and we can then change the dates and pass them to the DayPicker.

@unazko
Copy link
Contributor Author

unazko commented Dec 17, 2024

We need to remove the connection between CalendarLegend and DayPicker, currently they are tightly coupled and it's in contradiction with least knowledge principle(Law of Demeter). I would suggest removing the connection between them and move communication on Calendar level. We already pass the specialDates from Calendar to DayPicker, so we can now just use the same objects, add a private _tooltip property and have the tooltip(text property) from CalendarLegendItems. We can use attachInvalidate to listen for CalendarLegend invalidation and react upon, because apps can change the text or add more CalendarLegendItems and since the legend invalidates on child change, it will notify the Calendar as well and we can then change the dates and pass them to the DayPicker.

Done, as discussed. Turned out that the private property change did not cause invalidation.

@github-actions github-actions bot added the Stale label Jan 8, 2025
Copy link
Member

@Todor-ads Todor-ads left a comment

Choose a reason for hiding this comment

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

+1

@github-actions github-actions bot removed the Stale label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui5-calendar]: missing tooltips for the special dates
3 participants