Implement copy button #880
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…of the translations Since there are button settings in the "Navigation" section of buttons, it seems clearer to rename "Navigation" in the settings list to "Navigation and Buttons". There are many languages I have not added this to yet, but I can complete the translations soon.
Add new setting for showing/hiding the copy button in the output. Also add dividers between options to match other settings.
Implement copy button functionality. When copy button is clicked, output labels of currently selected symbols to clipboard
Create CopyButton component that follows BackspaceButton as template.
Add CopyButton to SymbolOutput to the left of backspace and clear. CopyButton is only displayed when enabled in navigationSettings (copyShowActive). CopyButton is also only displayed when at least one symbol has been selected (symbols.length > 0).
Add text to display for the setting that enables/disables showing the CopyButton. Note: these messages need translations!
martinbedouret
requested changes
Apr 9, 2021
There was a problem hiding this comment.
@aushaag Terrific job!! Congrats!
My only comment is that you don't need to worry about the translation files. You only need to add any new string to be translated into the cboard\src\translations\src\cboard.json file. before the production release, an automation script will take care of the translations.
Curly brackets are unnecessary
These are the descriptions next to the toggle to enable/disable the copy button in the output panel
I believe this will be overridden by the translation script anyway, but I realized there was a typo and "Display" should be "Navigation".
martinbedouret
approved these changes
Apr 10, 2021
sylvansson
reviewed
Apr 11, 2021
| @@ -179,6 +181,14 @@ export class OutputContainer extends Component { | |||
| this.clearOutput(); | |||
| }; | |||
|
|
|||
| handleCopyClick = () => { | |||
| const { intl, showNotification } = this.props; | |||
| let labels = []; | |||
There was a problem hiding this comment.
I know this was merged already, but I have a very small comment.
It would be simpler to assign the value when declaring the variable, and it's usually preferable to use forEach over map when we don't care about the function's return value.
const labels = this.props.output.map(symbol => symbol.label);There was a problem hiding this comment.
That makes sense. Just submitted a request for this update, thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Create new copy button in the symbol output (disabled by default). When users click the copy button, copy the labels of the selected symbols to the clipboard:
Disable CopyButton by default, and give users the option to enable it in Settings/Navigation/ShowCopyButton: