648628 - disable share button for a directed share type without a direction

Status: RESOLVED FIXED

(Cloud Services :: Share: Web Client, defect)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

in other words if you select twitter direct message but don't specific a recipient we should disable the share button.

Since I despise disabled buttons, it would be better to have a button that looks disabled and opens an alert when clicked with a message like: "Please enter a recipient for your share".

Comment 1

[Mike Connor [:mconnor]]

Wait... a UX person who prefers modal alerts over button disabling?  Crazy talk.

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

I might be cheating a bit here.  Since we don't yet have a "notification space" where I can easily put messages the JS alert should work since I'm assuming that with the new alerts (which are flat and embedded in the tab) it will remain within the panel.

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to comment #2)
> I might be cheating a bit here.  Since we don't yet have a "notification space"
> where I can easily put messages the JS alert should work since I'm assuming
> that with the new alerts (which are flat and embedded in the tab) it will
> remain within the panel.

Unfortunately you're assuming wrong. window.alert() from the panel browser will present an old modal alert in chrome.

I think we should do the error notification in content rather than surfacing a dialog.

Comment 4

[James Burke]

Attachment: Pointer to pull request

Comment 5

[James Burke]

The pull request review for this is actually just a pointer to the commit log for the hotfix branch: I should have done a separate branch of the hotfix branch to do this fix, but I goofed up.

With the changes, if no direct message recipient, the to field is highlighted with a red border and an an inline message is placed next to the disabled share button.

There is also checking for invalid recipients now, and if there is one, a similar treatment: red border highlight and inline message next to disabled share button.

Comment 6

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 525521 [details]
Pointer to pull request

nice!

Actual diff is at 

https://github.com/mozilla/f1/compare/master...hotfix%2F648372

That is a diff for the entire hotfix we'll be doing against the f1.momo live server.

Comment 7

[James Burke]

The changes have been merged into web/dev/1 which is used by the integrated Fx UI. With that merge, the fix is complete:
https://github.com/mozilla/f1/commit/bc0f61c89619c9504a0556c4780c897429888c48