Skip to content

Implement read-only useClipboard#201

Open
erayerdin wants to merge 2 commits into
childrentime:mainfrom
erayerdin:pr/use-clipboard-readonly
Open

Implement read-only useClipboard#201
erayerdin wants to merge 2 commits into
childrentime:mainfrom
erayerdin:pr/use-clipboard-readonly

Conversation

@erayerdin
Copy link
Copy Markdown

Description

Closes #200

Type of Change

  • Bug fix
  • New hook
  • Enhancement to existing hook
  • Documentation update
  • Other (please describe)

Checklist

  • I have read the Contributing Guide
  • My code follows the project's coding style
  • I have added tests for my changes
  • All existing tests pass
  • I have updated the documentation

The only thing left to do is to update the existing documentation. There are many, I mean, many markdown files. Some of them are for Docusaurus, some of them are for Astro. I was going to update it but I'm kind of confused. Can you inform me about how the docs build?

@childrentime
Copy link
Copy Markdown
Owner

Thanks for the PR! Two concerns:

  1. The option semantics look inverted. With options = { read: false } as default, read is false, so if (read) return is skipped and reading still runs — the default behavior is preserved by accident. But that means read: true actually disables reading, which is the opposite of what the name implies.

  2. To keep this non-breaking and intuitive, I'd suggest defaulting read to true (= "reading is enabled", current behavior), and letting users opt out with read: false when they only need writing:

    export const useClipboard: UseClipboard = (options = { read: true }) => {
      const read = options.read ?? true
      const updateText = useCallback(async () => {
        if (!read) return
        // ...
      }, [read])
    }

    Existing users keep working; users hitting the Chrome permission prompt ([Bug] A hook to only copy to clipboard #200) pass { read: false }.

Also: please add a test for the read: false path, and update the docs under packages/website-astro/src/content/docs/.

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.

[Bug] A hook to only copy to clipboard

2 participants