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

Move online status into modal #23130

Merged
merged 6 commits into from
Oct 2, 2020
Merged

Move online status into modal #23130

merged 6 commits into from
Oct 2, 2020

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Oct 1, 2020

Capture d’écran_2020-10-01_22-07-46
Capture d’écran_2020-10-01_22-12-15

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 1, 2020

Fixed a small bug on the button styling.
@juliushaertl the dashboard expect an Actions component and override the styling.
I had to copy/paste it so it fits as the .action-item__menutoggle selector you are using doesn't apply here anymore.

What do you wanna do? Extend your selector? Leave it like that? 🤷

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 1, 2020

I also found a bug that is unrelated I think. When you submit an empty message you don't get a warning stating the message is empty, it still tries to push the empty custom status message

@jancborchardt
Copy link
Member

Looks and works great, only the :focus doesn’t work – checking to see what’s wrong. Probably the focus is on a different element.

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 1, 2020

only the :focus doesn’t work – checking to see what’s wrong

The focus on what? If that is on the online radio selectors this is because there is no different design for checked/focused. Also the default browser is to use arrow keys for radio selectors. Not tab ✌️

@nickvergessen
Copy link
Member

I also found a bug that is unrelated I think. When you submit an empty message you don't get a warning stating the message is empty, it still tries to push the empty custom status message

Fixed that earlier #23108

@nickvergessen
Copy link
Member

The hint about notifications is missing for DND?

@jancborchardt
Copy link
Member

only the :focus doesn’t work – checking to see what’s wrong

The focus on what? If that is on the online radio selectors this is because there is no different design for checked/focused. Also the default browser is to use arrow keys for radio selectors. Not tab v

Aaaaah, keep forgetting about the arrow keys, jeez! Then all fine :D

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Tested and works very nicely!

@jancborchardt
Copy link
Member

The hint about notifications is missing for DND?

Yep, to fix the look then, "Invisible" could also get a subline saying "Appear offline" and then it would look nice. As discussed @skjnldsv

Also found another issue that the modal is not scrollable. The "Customize" modal of the Dashboard is, so we should do the same adjustment here. @juliushaertl did you do anything special there?

…mize

Signed-off-by: Jan C. Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan C. Borchardt <hey@jancborchardt.net>
@jancborchardt
Copy link
Member

The hint about notifications is missing for DND?

Yep, to fix the look then, "Invisible" could also get a subline saying "Appear offline" and then it would look nice. As discussed @skjnldsv

Also found another issue that the modal is not scrollable. The "Customize" modal of the Dashboard is, so we should do the same adjustment here. @juliushaertl did you do anything special there?

Fixed both of those issues!

Just needs the commit from @skjnldsv to show the subline.

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member

Bildschirmfoto von 2020-10-02 09-32-51

Subline added

@faily-bot
Copy link

faily-bot bot commented Oct 2, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 33546: failure

sqlite

Show full log
There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) OCA\Files_Versions\Tests\VersioningTest::testRestoreMovedShare
File content has not changed
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'version 2'
+'version 1'

/drone/src/apps/files_versions/tests/VersioningTest.php:729

mysql8.0-php7.4

  • cancelled - typically means that the tests took longer than the drone CI allows them to run

postgres9-php7.3

Show full log
There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testSearchPrincipal
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 END:VTIMEZONE\n
 BEGIN:VEVENT\n
 DTSTART;TZID=Europe/Berlin:20160419T130000\n
-SUMMARY:My Test (public)\n
-CLASS:PUBLIC\n
+SUMMARY:My Test (confidential)\n
+CLASS:CONFIDENTIAL\n
 TRANSP:OPAQUE\n
 STATUS:CONFIRMED\n
 DTEND;TZID=Europe/Berlin:20160419T140000\n
@@ @@
 LAST-MODIFIED:20160419T074202Z\n
 DTSTAMP:20160419T074202Z\n
 CREATED:20160419T074202Z\n
-UID:2e468c48-7860-492e-bc52-92fa0daeeccf.1461051722310-1\n
+UID:2e468c48-7860-492e-bc52-92fa0daeeccf.1461051722310-3\n
 END:VEVENT\n
 END:VCALENDAR'

/drone/src/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php:1272

@jancborchardt
Copy link
Member

Sitting here with @karlitschek and he says 👍 great job! :)

@rullzer rullzer merged commit 905e191 into master Oct 2, 2020
@rullzer rullzer deleted the fix/user-status branch October 2, 2020 09:24
@rullzer
Copy link
Member

rullzer commented Oct 2, 2020

/backport to stable20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to open status selection when scrolled down
4 participants