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

[ui polish] Reply pane styling/behavior #572

Closed
ninavizz opened this issue Oct 16, 2019 · 17 comments
Closed

[ui polish] Reply pane styling/behavior #572

ninavizz opened this issue Oct 16, 2019 · 17 comments

Comments

@ninavizz
Copy link
Member

ninavizz commented Oct 16, 2019

Mature Reply pane implementation to align with VisDe spec

  • Zeplin file here
  • Move leeeettle airplane button into window
  • Make airplane button BIGGER! (erm, sized as shown in spec)
  • STRETCH GOAL
    • Add keyboard shortcut so that when user hits Return key, send action is invoked
    • Add keyboard shortcut so that when user hits shift and Return keys, a new paragraph is added to their composed text
  • SUPER STRETCH GOAL
  • SUPER DUPER STRETCH GOAL:
    • Add static text to: to end Compose reply phrase
    • Add dynamic text %source_name% to follow static text, as shown in Zeplin mockup
@ninavizz ninavizz changed the title [ui polish] Compose Message pane [ui polish] Compose Reply pane Oct 16, 2019
@ninavizz ninavizz changed the title [ui polish] Compose Reply pane [ui polish] Reply pane styling/behavior Oct 16, 2019
@deeplow
Copy link
Contributor

deeplow commented Oct 20, 2019

Some inspiration on how to implement this with Qt might come from a Matrix client called "Nheko-Reborn". It's cpp but the layout structure can be similar. Take a look at the source code

@deeplow
Copy link
Contributor

deeplow commented Oct 22, 2019

I am now working on this. Here's the current version:

reply

@deeplow
Copy link
Contributor

deeplow commented Oct 22, 2019

@ninavizz I see that the spec does not account for a reply pane that can grow with the text like popular messenger applications. Was that considered at some point? Because it would make it much more readable not having a pane consuming 20% of the screen when nothing is typed

@deeplow
Copy link
Contributor

deeplow commented Oct 22, 2019

STRETCH GOAL

* Add keyboard shortcut so that when user hits `Return` key, send action is invoked

* Add keyboard shortcut so that when user hits `shift` and `Return` keys, a new paragraph is added to their composed text

So I was attempting do do this stretch goal but apparently it's more complex that I though:
because the user is in a text box, the Return keypress does not act as a shortcut

Because of that we probably have to make a subclass of QTextEdit that has a decorator for the that is called on key presses and then probably with signals, call the send_reply() function for the parent widget.

@eloquence eloquence added this to Nominated for next sprint in SecureDrop Team Board Oct 22, 2019
@eloquence
Copy link
Member

eloquence commented Oct 22, 2019

Add keyboard shortcut so that when user hits Return key, send action is invoked
Add keyboard shortcut so that when user hits shift and Return keys, a new paragraph is added to their composed text

While many messaging apps use this behavior, it's not clear to me that it's the best fit for SecureDrop. We have seen in user research that users don't apply a consistent paradigm to the SecureDrop client. Some map it against email, others against instant messaging.

If users follow the "email" mental model, seeing your message sent immediately after pressing [Enter] may be frustrating. Given the sensitivity of these communications, and the likelihood that many interactions may involve into multi-line messages, mirroring the expectation of email-like behavior may be a better fit for SecureDrop. In an email context, Ctrl+Enter is commonly used to "send immediately".

So an alternative approach would be:

  • Enter continues to behave as it does now (you can add paragraph breaks to your message)
  • Ctrl+Enter sends the message.

I would argue that this is the safer default behavior to avoid accidental replies.

Either way, we should also ensure that the user can [Tab] to the button and send the message that way.

@ninavizz
Copy link
Member Author

Damn. Fine. Catch. @eloquence!! Thank you for your attention to this detail, in catching quite a large error of my own judgement.

I agree, the "alternative" approach should be embraced, to prevent accidental replies from sending.

  • Enter continues to behave as it does now (you can add paragraph breaks to your message)
  • Ctrl+Enter sends the message.

I also agree, that Tab should work across the entire UI; but within the Reply pane, especially. <3

@deeplow
Copy link
Contributor

deeplow commented Oct 23, 2019

I didn't see any mockups for the offline mode, so I just did what I thought made sense.

The current version looks like this: (over in #580)

offline

@ninavizz I think we should also add a disabled-looking airplane. What do you think? (it the last iterations of the code there were none). With the plane disabled it would look like:

plane-disabled

I already have the code for that which I can merge into #580, in case that's the way to go.

@eloquence eloquence moved this from Nominated for next sprint to Current Sprint - 10/23 -11/6 in SecureDrop Team Board Oct 23, 2019
@eloquence
Copy link
Member

eloquence commented Oct 28, 2019

With #580 landed, I would suggest the following next round of changes to resolve this issue (in one or multiple PRs):

  • Ensure that source designation is bolded in #2a319d per the spec
  • Ensure that [Ctrl+Enter] sends a reply
  • Ensure that [Tab] sets focus to airplane, after which [Space]/[Enter] can be used to send a reply [1]
  • [Stretch] Ensure that entering the field clears out the placeholder text [2]

[1] I would call this stretch, except that with Ctrl+Enter as the shortcut to send messages, we really need a secondary way to quickly send replies for users who won't discover this shortcut.

[2] Qt's behavior here is consistent with how Chrome/Firefox handle the HTML5 placeholder attribute, though I agree w/ @creviera that in a single field form it's distracting.

@deeplow
Copy link
Contributor

deeplow commented Oct 28, 2019

@eloquence I'm investigating the following:

  • Ensure that source designation is bolded in #2a319d
  • [Stretch] Ensure that entering the field clears out the placeholder text [2]

@deeplow
Copy link
Contributor

deeplow commented Oct 28, 2019

* Ensure that [Tab] sets focus to airplane, after which [Space]/[Enter] can be used to send a reply  [1]

This is probably related with the qt5 property tabChangesFocus (docs)

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Oct 28, 2019

  • Ensure that source designation is bolded in #2a319d per the spec

This is being tracked in a smaller issue with links to relevant Zeplin screens: #593

Ensure that [Ctrl+Enter] sends a reply
Ensure that [Tab] sets focus to airplane, after which [Space]/[Enter] can be used to send a reply [1]

This is not being tracked in a smaller issue right now outside of this epic

[Stretch] Ensure that entering the field clears out the placeholder text [2]

This is being tracked in a smaller issue: #594

@deeplow
Copy link
Contributor

deeplow commented Oct 30, 2019

@ninavizz, what happens when the user is online, has typed something in the reply box already and then clicks in the "log out" button. Is the text supposed to disappear? Is there a warning dialog saying the text will be erased?

@ninavizz
Copy link
Member Author

Good question! There should be a modal warning dialog, here—a rare situation when a modal is a GOOD thing—however, I have not yet designed that. @eloquence should Deeplow finish this Issue w/o that warning, and have us add that warning in as a separate 'feature', later, or do you want him to pause while I throw that screen together?

@eloquence
Copy link
Member

should Deeplow finish this Issue w/o that warning, and have us add that warning in as a separate 'feature'

That sounds like a good plan. In the first implementation, I think it's fine for the text to be quietly discarded when you log out.

@deeplow
Copy link
Contributor

deeplow commented Oct 30, 2019

That sounds like a good plan. In the first implementation, I think it's fine for the text to be quietly discarded when you log out.

I'll do that, but it in another pr as currently the text stays as what the user typed

text

@eloquence
Copy link
Member

I've split the reply keyboard issue into a separate one (#606), and would propose splitting any remaining tweaks into separate smaller issues as well, closing this one.

@eloquence eloquence moved this from Current Sprint - 11/6-11/20 to Near Term - SD Workstation in SecureDrop Team Board Nov 20, 2019
@eloquence
Copy link
Member

Closing per proposal from above. We also have #351 if we really want a tracking epic.

SecureDrop Team Board automation moved this from Near Term - SD Workstation to Done Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

4 participants