Page MenuHomePhabricator

[native] Introduce disable link button

Authored by tomek on Mon, May 22, 6:32 AM.
Referenced Files
Sat, Jun 3, 5:33 AM
Unknown Object (File)
Fri, Jun 2, 7:18 AM
Unknown Object (File)
Thu, Jun 1, 11:37 PM
Unknown Object (File)
Mon, May 29, 11:41 PM
F558560: light-mode-disable.png
Thu, May 25, 7:04 AM
F551539: disable-button-bright.png
Mon, May 22, 8:21 AM
F551103: disable-button-confirm.png
Mon, May 22, 6:34 AM
F551102: disable-button.png
Mon, May 22, 6:34 AM



The button is visible only when a link is present.
Clicking the button results in alert being shown where a user is asked to confirm the action.
Confirming results in the link being disabled.

disable-button.png (2×1 px, 166 KB)

disable-button-confirm.png (2×1 px, 695 KB)

light-mode-disable.png (2×1 px, 171 KB)

Depends on D7907

Test Plan

Check if the button appears only when a link is present.
Check if clicking it results in an alert.
Check if canceling doesn't disable the link.
Check if confirming disables the link.

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

tomek requested review of this revision.Mon, May 22, 6:50 AM
140 ↗(On Diff #26775)

Hey @tomek! I took a look at our color.js file, and did notice that errorDark50 isn't used in any other components yet. I know the designs are using errorDark50, but thinking about it, it may be better to use

vibrantRedButton: designSystemColors.errorPrimary

My only concern with the primary color was how bright it may be and attention grabbing, but we already have components using the primary in our system. What do you think?

140 ↗(On Diff #26775)

For me there's a value in distinguishing between error and destructive actions. So having different colors for them makes sense. But merging these also is an option.

I've checked how it looks like with an alternative color. It's indeed brighter. But compared to the older color, that button looks a little bit like it's disabled. (but only in comparison)

disable-button-bright.png (2×1 px, 168 KB)

140 ↗(On Diff #26775)

@ted is on vacation now, but I agree the darker color looks a little bit disabled in comparison. Personally, I think I'd prefer the brighter red

kamil added inline comments.
125–131 ↗(On Diff #26886)

Wondering how this button looks in light mode

This revision is now accepted and ready to land.Thu, May 25, 4:33 AM
bartek added inline comments.
103–105 ↗(On Diff #26886)

Does this message look well in native alert box? It's pretty long

tomek added inline comments.
103–105 ↗(On Diff #26886)

In the summary there's a screenshot with it - looks ok, I guess.

125–131 ↗(On Diff #26886)

Included a screenshot in the summary

This revision was automatically updated to reflect the committed changes.