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

put a small message "Copied" when message or url is copied into clipboard #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jsspc
Copy link
Collaborator

@jsspc jsspc commented Oct 21, 2020

Pull Request

Description

Created clipboard.dart to create function to handle text being copied to clipboard in 2 contexts (copy message, copy URL).

Trello Card

https://trello.com/c/JETCwEo6

Checklist

Check the requirements you have fulfilled

I have:

  • Named my PR from the name of the trello card
  • put meaningful comments in my code
  • Refactored my code
  • Added a description in the PR description
  • Added the link to the trello card in the PR's description
  • Added the link of the PR in the Trello Card
  • Included some screenshots when I changed the UI
  • Tested the app

image

Looks like this, will always display at the bottom of the screen (excluding keyboard if it is active).

@ValentinVignal
Copy link
Owner

Sorry I didn't write it in the card, but maybe the message "Copied" is better (when you copy a url it is not a message) and it is smaller.

@ValentinVignal
Copy link
Owner

Do you think it is possible to have

  • The text centered
  • Some space at the bottom (so the buble is not too low)
  • shrink the width (so there is no extra space at the right and at the left)

@@ -266,7 +268,9 @@ class _MessageTileState extends State<MessageTile> {
color: const Color(0x00000000),
icon: Icons.content_copy,
foregroundColor: Colors.grey,
onTap: () => Clipboard.setData(ClipboardData(text: widget.message.text)),
onTap: () {
copyToClipboard(widget.message.text).show(context);
Copy link
Owner

Choose a reason for hiding this comment

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

I think it is better if you do a function like that copyToClipboard(context, widget.message.text); (with context as the first argument)

import 'package:flutter/services.dart';
import 'package:flushbar/flushbar.dart';

final flushbarCopied = Flushbar<dynamic>(
Copy link
Owner

Choose a reason for hiding this comment

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

I'm would have instanciated the Flushbar Object in the function but okay for this one

import 'package:flutter/services.dart';
import 'package:flushbar/flushbar.dart';

final flushbarCopied = Flushbar<dynamic>(
Copy link
Owner

Choose a reason for hiding this comment

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

Try to avoid dynamic type as it doesn't give any information. So replace this with a real type (preferred option) or don't put it

flushbarStyle: FlushbarStyle.FLOATING,
borderRadius: 20,
isDismissible: true,
duration: Duration(milliseconds: 20),);
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't 20 milliseconds too short ?

isDismissible: true,
duration: Duration(milliseconds: 20),);

Flushbar copyToClipboard(String textToCopy) {
Copy link
Owner

Choose a reason for hiding this comment

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

As said previously, take the context in the function as a first argument and call show() in the function

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.

2 participants