-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[scheduler][styled] Add more events popover #19880
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
base: master
Are you sure you want to change the base?
Conversation
Deploy preview: https://deploy-preview-19880--material-ui-x.netlify.app/ Bundle size report
|
className={clsx('MonthViewContainer', 'mui-x-scheduler', className)} | ||
{...other} | ||
> | ||
<EventPopoverProvider containerRef={containerRef}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a 3rd provider in the future, we should create a single <ViewProvider />
component to wrap them.
I'm fine keeping the explicit 2 provider for now 👍
import { createPopoverComponents } from '../popover'; | ||
import './EventPopover.css'; | ||
|
||
const EventPopoverComponents = createPopoverComponents<CalendarEventOccurrence>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const EventPopoverComponents = createPopoverComponents<CalendarEventOccurrence>({ | |
const EventPopover = createPopover<CalendarEventOccurrence>({ |
WDYT? Since there is also a hook inside the returned value
</div> | ||
<div className="MoreEventsPopoverContent"> | ||
{occurrences.map((occurrence) => ( | ||
<AgendaEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rename this component if we use it outside of AgendaView
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I would imagine this component used in a lot of places - since it does not care who its parent component is, and does not care about positioning.
What would you name it? A few suggestions:
EventItem
EventCard
(We are calling the class of all the events .EventCard
so not sure)
EventListItem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine starting with <EventItem />
with a good JSDoc.
EventCard (We are calling the class of all the events .EventCard so not sure)
Side note, I think we should remove any code that is inside a specific event CSS file but targeting a generic class (like .EventCard
) because it makes CSS side effect super easy to do by mistake.
packages/x-scheduler/src/internals/components/popover/Popover.tsx
Outdated
Show resolved
Hide resolved
packages/x-scheduler/src/month-view/month-view-row/MonthViewCell.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Flavien DELANGLE <[email protected]> Signed-off-by: Nora <[email protected]>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
closes #17722
In this PR:
MoreEventsPopover
(naming debatable) to show hidden events in the month viewNot in this PR
MoreEventsPopover
#19879