quadratichq / quadratic

Quadratic | Spreadsheet with Python, SQL, and AI
https://QuadraticHQ.com
Other
3k stars 151 forks source link

Clicking around when context menu is up should close menu, and select cell under pointer #153

Closed deshumake closed 1 year ago

deshumake commented 1 year ago

In RightClickMenu.tsx, there is this div in the beginning of the return to ensure that the onClose() method is called

export const RightClickMenu = (props: RightClickMenuProps) => {
  return (
    <>
      <div
        style={{
          position: 'absolute',
          top: '0',
          left: '0',
          width: '100%',
          height: '100%',
          zIndex: '1000',
          display: props.state === 'open' ? 'block' : 'none',
        }}
        onClick={(event) => {
          props.onClose({ reason: 'blur' });
        }}
      ></div>

Left click problem: I tried deleting this div so that clicks would go through to the actual grid, and the context menu still closes. Is this div really necessary then?

Right click problem: Additionally, clicking with the right mouse button should also select a cell, so that copying and pasting work as expected.

That issue is being caused by this chunk of code in the onPointerDown() of usePointerEvents.ts

153 |  const rightClick = event.button === 2 || (event.button === 0 && event.ctrlKey);
154 |  
155 |  // If right click and we have a multi cell selection.
156 |  // If the user has clicked inside the selection.
157 |  if (rightClick && props.interactionState.showMultiCursor) {
158 |    if (
159 |      down_cell_x >= props.interactionState.multiCursorPosition.originPosition.x &&
160 |      down_cell_x <= props.interactionState.multiCursorPosition.terminalPosition.x &&
161 |      down_cell_y >= props.interactionState.multiCursorPosition.originPosition.y &&
162 |      down_cell_y <= props.interactionState.multiCursorPosition.terminalPosition.y
163 |    )
164 |      // Ignore this click. User is accessing the RightClickMenu.
165 |      return;
166 |  }
167 |  
168 |  // otherwise ignore right click
169 |  else if (rightClick) {
170 |    return;
171 |  }

Could someone give me some more context for this function?

The functionality I'm proposing in the Right Click Problem above is resolved by deleting lines 169-171

davidkircos commented 1 year ago

@deshumake sounds like a good change, could you make a PR? If you do so make it off of the branch development

deshumake commented 1 year ago

@davidkircos I made the PR for adding the functionality described above, but the questions about the code still stand if anyone knows

davidkircos commented 1 year ago

@deshumake check out our new context menu and let me know if you believe this issue is now solved. https://app.quadratichq.com/

Screenshot 2023-02-21 at 8 54 54 PM
deshumake commented 1 year ago

This looks and works beautifully!