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

Propose Clipboard Interaction specs #90

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Propose Clipboard Interaction specs #90

merged 1 commit into from
Apr 12, 2023

Conversation

seancolsen
Copy link
Contributor

@seancolsen seancolsen commented Apr 3, 2023

@seancolsen seancolsen added this to the Backlog milestone Apr 3, 2023
@seancolsen seancolsen added the status: review In review label Apr 3, 2023
@seancolsen
Copy link
Contributor Author

@pavish and @ghislaineguerin I have assigned you to review these proposed UX specs for the Clipboard Interaction design as motivated by mathesar-foundation/mathesar#2377.

I've assigned you two since you're designated helpers on the Usability Improvements project which contains this work.

I also have a draft PR which begins to implement the "copy" functionality.

@pavish and @Anish9901 I'll highlight that this design work seems to have some overlap with the UI for Importing data into existing tables GSoC project, which you are mentoring.

Copy link
Member

@pavish pavish left a comment

Choose a reason for hiding this comment

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

@seancolsen The spec looks good to me.

Comment on lines +291 to +295
- It would be nice to add **context menu entries for copy/paste**, but there are some challenges here.

- When copying: If we use the synchronous clipboard API it seems[^1] that we can't trigger copy events programmatically. Somehow Google Sheets is doing it, but I haven't dug deeper to figure out how. With the async clipboard API Firefox lacks support for MIME types other than `text/plain`.

- When pasting: The synchronous clipboard API doesn't allow programmatic pasting at all. The async clipboard API doesn't work allow pasting in Firefox and requires a special permission in other browsers.
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the clipboard API support this?

https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText

We could use navigator.clipboard.writeText() method to write to the clipboard, the only caveat being Firefox only supports this event from a user action, which will be the case for context menus.

For paste, we'd be reading from the clipboard using the navigator.clipboard.readText() method, and performing "paste" manually i.e. updating the cells ourselves.

I remember doing this a long time ago and there were some challenges like the elements need to be in focus.

I'm adding this comment here, perhaps we could update the document with this as well, so that when we want to implement this, this comment can be a starting point.

Copy link
Contributor Author

@seancolsen seancolsen Apr 11, 2023

Choose a reason for hiding this comment

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

@pavish Firefox supports writeText but it doesn't support write, and I want to use write so that I can write clipboard data with custom MIME types. Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

It does. Perhaps for Firefox we can limit support to text/plain and try working with that, and/or we could write items to localstorage as a way to improve copy/paste support within Mathesar.

Either way, I agree this isn't a high priority at the moment, and we can consider picking it up later.

@pavish pavish removed their assignment Apr 11, 2023
@ghislaineguerin
Copy link
Contributor

@seancolsen The spec looks good. Regarding your design notes, I think having no contextual menu option is fine for now since content of our cells is very consistent with no styling, so we can do without paste options. I'm assuming that's what you meant by "How to Paste." Is that correct? What other paste options did you had in mind?

One feature I find very helpful is the ability to paste a list of text with line breaks, creating new rows for each entry. I'm not sure if that's a consideration, but it's something we should explore in the future.

@seancolsen
Copy link
Contributor Author

@ghislaineguerin

  • I'm assuming that's what you meant by "How to Paste." Is that correct?

    No. What I meant by "How to paste" is this... If we wanted a context menu option for pasting, then implementing it nicely would be a challenge, given the limitations of Firefox. We could work around these limitations by displaying a modal dialog when the user chooses a "Paste" action from the context menu. The modal should show content which instructs the user to paste via their keyboard shortcut. Google Sheets does this:

    image

    The "How to paste" modal dialog is an implementation strategy, not really a design choice. Ideally, we would just be able to read data from the users clipboard after the user selects the "Paste" context menu option, but unfortunately it's not that easy (especially in Firefox).

  • What other paste options did you had in mind?

    I don't have any options in-mind to present the user before pasting. Note that the specs do require that we display a modal which asks the user to "Paste Raw Values" or to "Paste Formatted Values", but that modal displays after the paste action is initiated, and it only displays under certain circumstances which I predict will not be very common.

  • One feature I find very helpful is the ability to paste a list of text with line breaks, creating new rows for each entry.

    Yes. The behavior you describe is already required within these specs. Perhaps the spec should be more explicit about that. I'm not sure it's worth the time to make more edits though, since I predict I'll be the one to implement this and it seems like we're all in agreement about the behavior.

    When pasting plain text, the text is deserialized as TSV data. That deserialization logic is subject to some improvisation during implementation (as described in the "Deserialization" section).

    • Simple cases like this will deserialize into three separate cells:

      apple
      banana
      cherry
      
    • It gets a little more tricky is when cells are quoted. This should deserialize into two cells:

      "apple
      banana"
      cherry
      

      That behavior is consistent across Google Sheets and LibreOffice Calc

    • But cases like this have inconsistent behavior across Google Sheets and LibreOffice Calc, so we'll need to improvise these during implementation (perhaps deferring to the behavior a third party TSV parsing library)

      ""apple
      banana"
      cherry
      

@seancolsen
Copy link
Contributor Author

I'm going to merge this now since it's seems like we have sufficient approval. We can continue to tweak smaller details as necessary.

@seancolsen seancolsen merged commit 9b948e6 into master Apr 12, 2023
@seancolsen seancolsen deleted the clipboard-specs branch April 12, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants