UX improvements change status dialog #23305
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
Regarding compiled assets, I had to guess by looking at other PRs. |
| {{ $t('user_status', 'Clear status message') }} | ||
| </button> | ||
| <button class="status-buttons__primary primary" @click="saveStatus"> | ||
| <button class="status-buttons__primary primary" :disabled="isSavingStatus || !message" @click="saveStatus"> |
Member
There was a problem hiding this comment.
Suggested change
| <button class="status-buttons__primary primary" :disabled="isSavingStatus || !message" @click="saveStatus"> | |
| <button class="status-buttons__primary primary" :disabled="isSavingStatus" @click="saveStatus"> |
Setting to empty clears the status just fine, might be less confusing
Member
Author
There was a problem hiding this comment.
It didn't in my instance and caused a server error as it was sending a status icon + empty message.
I personally wanted to set a status icon without message at that time, this is how I found the issue. Another user might think likewise. So if we fix it to clear the status on empty, the icon would not be set and would not match the user's expectation here.
Member
Author
There was a problem hiding this comment.
ok, then it's either a regression or I found an action path that doesn't work:
I'll split this into a separate PR and keep this PR here for just the enter key improvements
PVince81
force-pushed
the
bugfix/noid/change-status-ux-fixes
branch
from
55f5522 to
73cdb88
Compare
Member
Author
|
I've now removed the code related to empty messages. The disabled attribute is still there but only for disabling the button when saving as we do in other places. Adjusted and squashed. |
Member
Author
|
I've raised #23411 for the empty status issue |
PVince81
force-pushed
the
bugfix/noid/change-status-ux-fixes
branch
from
73cdb88 to
ead50ec
Compare
Member
Author
|
I did rebuild and check in the assets. Strange. After rerunning |
PVince81
force-pushed
the
bugfix/noid/change-status-ux-fixes
branch
from
72740c8 to
997ce7c
Compare
Member
Author
|
ouch, pushed on wrong branch... fixing... |
Member
Author
|
okay, I decided to push the empty message fix here as well to avoid needless future conflicts. Fixes #23411 |
jancborchardt
approved these changes
Oct 13, 2020
Member
jancborchardt
left a comment
jancborchardt
left a comment
There was a problem hiding this comment.
Good stuff design-wise, great work @PVince81! :)
Member
Author
|
there was a weird unrelated error: build restarted |
Member
Author
|
@nickvergessen CI green, can you review teh codez ? |
rullzer
requested review from
ChristophWurst,
georgehrke,
juliusknorr and
skjnldsv
Member
|
@PVince81 could you rebase so we can get this in? |
ChristophWurst
approved these changes
Oct 20, 2020
skjnldsv
approved these changes
Oct 20, 2020
When selecting an emoji but not specifying a message, the internal message value was null which caused a server validation error on sending. This fixes the SetStatusModal to always work with an empty string upfront, as this is the value the field would have if edited and cleared manually. Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Focus on the custom message field after picking an emoji. Hitting the enter key while in the custom message field now triggers saving. Disable save buttons while saving is in progress. Signed-off-by: Vincent Petry <vincent@nextcloud.com>
PVince81
force-pushed
the
bugfix/noid/change-status-ux-fixes
branch
from
2d6a50b to
08813f5
Compare
Member
Author
|
Rebased and rebuilt the module in question, as it was the only conflict. |
|
🤖 beep boop beep 🤖 Here are the logs for the failed build: Status of 34374: failurenodbShow full logacceptance-app-files-sharing
Show full log |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Focus on the custom message field after picking an emoji.
Enter key in the custom message field saves the status.
Disable the save button when no custom message is entered (would resultin a server error).
Disable save buttons while saving is in progress.
Fixes setting an empty message for clearing when not touching the field (#23411)
Motivation
~~
This was mostly because I noticed that entering no custom message triggered an error. I didn't manage to make the server accept empty messages (and even then who knows what side effects...), so I went with prevent empty status message.
And as a bonus added the enter key + autofocus.
~~
Issues
Partial fix for #23276 by introducing "save by enter key"
Fixes #23411