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

Change design of account notes in web UI #14208

Merged
merged 2 commits into from
Jul 6, 2020
Merged

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Jul 3, 2020

Peek 2020-07-03 21-23

  • Can press ctrl+enter to save and unfocus
  • Can press esc to reset and unfocus
  • Saves on blur or unmount

@Gargron Gargron force-pushed the fix-account-notes-design branch 2 times, most recently from 81dec9a to 8fafa26 Compare July 3, 2020 19:32
@brawaru
Copy link
Contributor

brawaru commented Jul 3, 2020

Wow! That's nice and seamless, how does it look in light theme?

@brawaru
Copy link
Contributor

brawaru commented Jul 3, 2020

Also,

Can press ctrl+enter to save and unfocus

Why not just adapt Enter for this, but allow newline with Shift+Enter?

Can press esc to reset and unfocus

That would be unfunny to discover on random. Maybe it's best to remove it altogether? Esc behavior is already unclear (sometimes it closes modals, sometimes it cancels page load).

@Gargron
Copy link
Member Author

Gargron commented Jul 3, 2020

Why not just adapt Enter for this, but allow newline with Shift+Enter?

Ctrl+Enter works as save/submit in multiple other forms so it seems like what most users would expect to just work.

Can press esc to reset and unfocus

That would be unfunny to discover on random. Maybe it's best to remove it altogether?

Not sure, but it makes sense to me to have a quick reset hotkey.

@SuperSandro2000
Copy link
Contributor

Can press ctrl+enter to save and unfocus

This is the default on many other website with similar forms including Github

Not sure about the reset on Esc but the unfocus is intuitiv.

@Gargron
Copy link
Member Author

Gargron commented Jul 3, 2020

I doubt anyone would have a problem with it but if anyone does I'll change it in the next update

@ClearlyClaire
Copy link
Contributor

It does look better, but I'm not sure if it's as intuitive or usable. It'd be the first thing we have so far that you just click to edit, and there's nothing telling you that unfocusing it will save it or anything.
Also, making it sit between two parts that are entirely defined by the account owner feels a bit weird and confusing.

@Gargron
Copy link
Member Author

Gargron commented Jul 4, 2020

Actually, the media descriptions used to be like this! So even though we changed them into the media editing modal, there's precedent. I've tried different combinations and I think it sits here the best!

@ClearlyClaire
Copy link
Contributor

Oh, I remember the media editing thing being an issue on mobile because of that: it was difficult discovering you could actually do that. The “Click to add a note” placeholder probably is enough to address that issue though.

@ClearlyClaire
Copy link
Contributor

Yeah, it does look better, but my complaints with it are:

  • it's not completely obvious that you can edit the note by clicking on it (that concern is mostly addressed by the “Click to add a note” placeholder, but that placeholder won't be there when you have a note already, from, e.g. a moved blocked person, or by adding a note yourself, from the WebUI, or from a third-party client that may have a different UI for this)
  • the lack of “save” button will probably confuse a lot of users at first, especially since it's the only thing that works like this in the UI right now
  • the lack of feedback regarding whether it's successfully saved or not

@ClearlyClaire
Copy link
Contributor

What about keeping the same style as this PR, but initiating the edit with a pencil button or something besides the “Note” header, and having the “Save” / “Cancel” button from previously? They would increase the height of the note part of the UI, but not change how the input itself would look.

@brawaru
Copy link
Contributor

brawaru commented Jul 5, 2020

@Gargron
Ctrl+Enter works as save/submit in multiple other forms so it seems like what most users would expect to just work.

Not really, no. In context of [non-retractable] forms, you may be required to enter multiple lines, there this would be suitable. For user notes, if we adapting such quick edit field, I think save shortcut must be simplified too. What are the odds user is going to insert multiple line breaks on a short note? Pretty low, probably. “And if you are exceptional one, we give you Shift+Enter” (sound effect).


@ThibG
it's not completely obvious that you can edit the note by clicking on it

Very good point and not easy to solve, we already have a pointer, which changes to beam on hover, but maybe can make it ever more noticeable by adding a border on hover (which is massive area).

Like this

GIF demonstrating border appears when cursor hovers over textarea, when textarea is focused, border color matches its background

Or, using hint (in the far far away future):

Like this

GIF demonstrating hint element right to the note heading, once pressed, tooltip appears “Notes on accounts. Create notes visible only to you. Click on note to edit it, changes saved automatically.”


Regarding save / cancel buttons, I don't see any benefit of them. The notes feature is dead simple, as a notepad — it's only you (and instance admin, but shhh) who sees them, and you probably not going to put very important stuff there, so you don't really need to worry about submitting it. So yea, saving on unfocus works pretty well in such seamlessly integrated textarea. Remove resetting and debounce saving on type and users will know it's saving as they type, because:


Also,

the lack of feedback regarding whether it's successfully saved or not

GIF demonstrating “Saved” indicator appears every time note is saved (along with 500 Internal Server Errors, which suggests Sasha did something wrong on server side)

<details>

I took some time to rewrite the whole class component to functional one, just for fun, you know. And thought, why shouldn't I try and implement my concept? Umm, yea… Regarding 500 errors, it's my developer instance slightly broken with me jumping from one branch to another, and this actually makes a good point that I need to determine whether it saved successfully or not, don't think it's much of a problem. I may post gist with the code, but it's too much hooking, you may not like this. Okay, I posted it.

P.S. The border on GIF is React DevTools.

@ClearlyClaire
Copy link
Contributor

What are the odds user is going to insert multiple line breaks on a short note? Pretty low, probably.

Probably low, yes, but not extremely so, and the user would need a way to find out that they can indeed do that.
Everywhere else, when multiple lines are possible, you simply press “enter” to achieve them. You'd basically need knowledge of other software to know that shift+enter would allow you adding a newline.

Very good point and not easy to solve, we already have a pointer, which changes to beam on hover, but maybe can make it ever more noticeable by adding a border on hover (which is massive area).

Those things only work when you have a mouse pointer, not when you're on mobile.

Regarding save / cancel buttons, I don't see any benefit of them. The notes feature is dead simple, as a notepad — it's only you (and instance admin, but shhh) who sees them, and you probably not going to put very important stuff there, so you don't really need to worry about submitting it. So yea, saving on unfocus works pretty well in such seamlessly integrated textarea. Remove resetting and debounce saving on type and users will know it's saving as they type, because:

Yeah, the feature is simple, but you need to know how it'll be saved, and I think you'd need to be able to cancel a change too. Currently, you can do that by pressing Escape. That doesn't work on mobile.

@brawaru
Copy link
Contributor

brawaru commented Jul 5, 2020

@Sasha-Sorokin
I'll come up with other concept a bit later, I hope.

Updated previous comment with another concept (see “using hint” spoiler).

@Sasha-Sorokin
I may post gist with the code

Also added a link!

@Gargron Gargron force-pushed the fix-account-notes-design branch from 8fafa26 to f0abaae Compare July 5, 2020 16:02
@Gargron
Copy link
Member Author

Gargron commented Jul 5, 2020

  • Fixed some bugs around accidentally erasing note while the web UI loads it
  • Removed blur on ctrl+enter because the consequent blur event sends a 2nd save request really fast and that's not nice
  • Added "Saved" inline alert from @Sasha-Sorokin's suggestion

Edit: Solved the issue of blur firing a 2nd save after ctrl+enter by entering into a "saving" state that's reset when the prop value is updated to the state value (i.e. the server-side value saved successfully) or when the user changes the state value again.

@Gargron Gargron force-pushed the fix-account-notes-design branch 3 times, most recently from 6a467d5 to 6fc0baa Compare July 5, 2020 16:21
@Gargron Gargron force-pushed the fix-account-notes-design branch from 6fc0baa to 018b35f Compare July 5, 2020 16:23
@Gargron Gargron requested a review from ClearlyClaire July 5, 2020 21:29
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Fully went through the code, and I haven't seen any real issue with it, but I think the component being stateful and trying to handle context changes make it significantly more difficult to understand, tbh.

Design-wise, I think it looks significantly better than the previous version, but I'm still not convinced it's as easy to use, particularly on mobile, where you don't really have a way to cancel your changes. Maybe those concerns aren't that important though 🤷‍♀️

@Gargron Gargron merged commit c318741 into master Jul 6, 2020
@Gargron Gargron deleted the fix-account-notes-design branch July 6, 2020 23:24
@mayaeh mayaeh mentioned this pull request Jul 7, 2020
@@ -323,6 +314,8 @@ class Header extends ImmutablePureComponent {
</div>
)}

{account.get('id') !== me && <AccountNoteContainer account={account} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, why did you remove ability to set note for yourself?

Copy link
Member Author

@Gargron Gargron Jul 7, 2020

Choose a reason for hiding this comment

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

Because we don't load relationship for the user itself the note is never loaded, so while it was possible to set it, it would always show up as empty later.

Copy link
Contributor

@brawaru brawaru Jul 7, 2020

Choose a reason for hiding this comment

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

Is it possible to fix it somehow? Because I find note for myself useful, just like chatting with myself in messengers… you can leave some things for later in there. I actively used that in Discord.

ClearlyClaire added a commit to ClearlyClaire/mastodon that referenced this pull request Jul 8, 2020
@blank71
Copy link

blank71 commented Jul 9, 2020

Add keyboard shortcut is nice. But I would like you to add some buttons, like previous one.
#14148 (comment)

Current one is not user-friendly at mobile and for mastodon beginner.
I tried both, previous one and current one. Current one quite confused me to add note, because there are no any buttons. At this point previous one is nice.

Mastodon settings has "save" button and setting is not saved automatically. This is my thought that Mastodon base design is saving manually not saving automatically. So add "save" button is natural for me.😊

shouo1987 pushed a commit to CrossGate-Pawoo/mastodon that referenced this pull request Dec 7, 2022
* Change design of account notes in web UI

* Fix `for` -> `htmlFor`
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.

5 participants