WIP: custom actions context menu #247
Conversation
UI implementation of context menu, no business logic yet. WIP.
…s styling be consistent in UI Purpose is consistency. any inconsistent border radius, even by small pixel differences, will look "off" to the user.
that-one-arab
changed the title
Purpose is to remain consistent across codebase
that-one-arab
force-pushed
the
feature_custom_actions_context_menu
branch
from
37f0b1e to
e5ecea7
Compare
Detecting whether an event was right clicked or not in a clean manner solves a big hurdle.
Not sure why it was of type button in the first place, but this fixes it.
This commit demonstrates a POC for editing an event and inner functionality of how the context menu will interact with an event in its draft state.
that-one-arab
force-pushed
the
feature_custom_actions_context_menu
branch
from
de277d4 to
5acb05d
Compare
the purpose is to prevent an additional action from occurring when user tries to close the context menu by clicking outside. Mimicking user expecations.
|
Hey @that-one-arab , I see you're finding solutions to make this work. But I'm concerned that we're reinventing the wheel and adding unnecessary complexity along the way. Specifically, calculating the x,y coordiantes, creating context menu, and adding an invisible backdrop. Please check out floating-ui, which can solve a lot of these problems out-of-the-box and in a more reusable fashion. We're already using this for the forms, and could use it for other elements like tooltips (for shortcuts) and select menus (for time inputs). Although what you have so far might work, I'd rather go with the more resusable solution that requires less code and maintenance. If we can use a good library like floating UI to allow us to just focus on the unique aspects of a context menu in Compass (its styling and elements), that'd be a big win |
Agreed. I wasn't aware of the library, I am checking it out and I like the reliability of it and the complex positioning scenarios it handles out of the box. I will refactor the PR accordingly. |
Floating UI is a well tested library that handles a lot of nuances and edge cases with floating elements positioning
| @@ -47,6 +47,8 @@ export const MainGridEvents = ({ measurements, weekProps }: Props) => { | |||
|
|
|||
| const editTimedEvent = (event: Schema_GridEvent) => { | |||
| dispatch( | |||
| // TODO: `actions.startDragging` is confusing, it appears to mix concerns for both dragging | |||
There was a problem hiding this comment.
Agreed. Some of my changes from my current draft PR will improve how this works
There was a problem hiding this comment.
PS This comment is just be a harmless note-to-self in a draft PR, but it's a good opportunity to share my thoughts on comments in general: https://docs.compasscalendar.com/docs/contribute/convention-guide#comments.
Go ahead and keep using TODO comments during your WIP PRs if it helps your workflow, but please remove them before submitting the PR. If they're still relevant at the time of publishing your PR, you could add an issue or inline PR comment like I'm doing here so we don't miss out on your thoughts
There was a problem hiding this comment.
@tyler-dane Nice catch yes I wasn't aware about the comments policy, and I happen to agree to your philosophy regarding it. I'll make sure to clean it up.
- Replace `event` local state with draft event redux state as the source of truth for the selected event - Handle setting and clearing the right-clicked event as the draft event - rename `event` to `gridEvent` to avoid confusion with DOM events - TS typing and error fixing
…ent is right clicked This is a crucial line. It allows us to handle right clicks separately (EG: Displaying a context menu)
- Implement context menu edit priority - Decouple `ContextMenu` and create `ContextMenuItems` to separate floating ui menu logic from menu items business logic
No description provided.