mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.14k stars 2.91k forks source link

FXIOS-602 ⁃ [Today Widget] Go to copied link #6935

Closed brampitoyo closed 4 years ago

brampitoyo commented 4 years ago

Why/User Benefit/User Problem

Behaviour

Visual Design

“Go to copied link” should look like another circle button – aligned with the other three widget items.

Size: 5

┆Issue is synchronized with this Jira Task

brampitoyo commented 4 years ago

CC @noorhashem & @nbhasin2.

noorhashem commented 4 years ago

@brampitoyo so after discussions with Nishant, we see that we can only read the clipboard for URLs once the user clicks the button so the action is user-initiated and we take their consent to read into the clipboard instead of reading it every time the view re-appear. the notification will keep on showing on any occasion you read the clipboard so it's unavoidable.

noorhashem commented 4 years ago

@brampitoyo can you send me the assets for the new button please?

brampitoyo commented 4 years ago

@noorhashem We have a similar-looking asset currently used in our Today View Widget to indicate “Go to copied link”:

Let’s reuse this icon and not come up with a new one.

noorhashem commented 4 years ago

@brampitoyo can I have the pdf file as you did with the search and private search last time with the rounded button ? I only have the standalone copy icon right now, I can make the rounded button programmatically but would be faster and more polished to just use the asset.

brampitoyo commented 4 years ago

@noorhashem For sure! Sorry for the delay. Here’s the PDF file for this asset.

go-to-copied-link.pdf

noorhashem commented 4 years ago

@brampitoyo thanks a lot! I noticed that in the new dark mode mockups, you had the icons in white -> turn into a black version (new search & copied link) can you send me the dark versions as well, since the last mockup had the same icons in both light/dark mode so i didn't change icons when modes changed, just changed texts.

athomasmoz commented 4 years ago

@brampitoyo What should happen where there is no URL in the clipboard? (ie. don't show button if no URL in clipboard / show notification if clicked)

brampitoyo commented 4 years ago

@noorhashem I see!

Would you like dark icons for the search asset, as well? Sorry, I thought that you’d be able to easily change the value!

They are both here:

@athomasmoz When Noor and I talked last week, we think that we can make the icon disappear (e.g. don’t show asset) if there's nothing in the toolbar.

Now, I’m not so sure anymore. If the act of reading the clipboard will cause a system notification to appear (“Firefox pasted from [App_Name], do we still want to do it in order to determine whether to show the icon or not?

If we can’t do it in a way that doesn’t make people feel “scared”, I propose showing this “Go to link” button at all times. If there is nothing on the clipboard, then we simply do nothing. It feels kind of “dumb” that way, but we may also avoid the need to read the clipboard until the point where we really need it (ie. when Firefox opens and the clipboard content needs to be pasted).

@noorhashem @nbhasin2 Is there a way for us to determine whether the clipboard is empty or not empty, without triggering the system notification?

noorhashem commented 4 years ago

@brampitoyo Thanks bram. I could only change values if I had transparent Icons with no bakcground and just outlines, I can then invert the colors, but I couldn't with the current designs.

as for the button status in case nothing was on clipboard, there is a function that can check if there's nothing in the clipboard or not and it doesn't invoke the notification. we'll use that and hide it if there is nothing in clipboard and test it out.

brampitoyo commented 4 years ago

[…] there is a function that can check if there's nothing in the clipboard or not and it doesn't invoke the notification. we'll use that and hide it if there is nothing in clipboard and test it out.

@noorhashem That sounds excellent. Thanks!

SimonBasca commented 4 years ago

Verified fixed on 28.0 (18809). Tested on iPhone XS Max (14.0b2), iPhone Xr (13.5), iPhone SE (11.4).