Skip to content

Add current status in navbar#29210

Merged
CarlSchwan merged 1 commit intomasterfrom
work/status-navbar-vue
Oct 15, 2021
Merged

Add current status in navbar#29210
CarlSchwan merged 1 commit intomasterfrom
work/status-navbar-vue

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Oct 13, 2021 •
edited
Loading

This ports the code to Vue and if the user_status app is disabled this
goes back to the previous behavior.

The Avatar vue component detects changes of the status automatically.

image

Signed-off-by: Carl Schwan [email protected]

@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from da53431 to c551db0 Compare October 13, 2021 10:41
@CarlSchwan CarlSchwan requested review from a team, PVince81, artonge, jancborchardt and szaimen and removed request for a team October 13, 2021 10:42
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from c551db0 to 282bec1 Compare October 13, 2021 10:51
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

@nickvergessen
Copy link
Member

nickvergessen commented Oct 13, 2021

Tooltip seems to not live update?
Bildschirmfoto von 2021-10-13 12-55-10

Should subscribe to:
https://github.com/nextcloud/spreed/blob/98d679964a2352a4fa455c514944273a2b14b778/src/components/RightSidebar/Participants/CurrentParticipants/CurrentParticipants.vue#L89-L95

jancborchardt
jancborchardt requested changes Oct 13, 2021
View reviewed changes
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.

Very nice! :) One thing: We can go without the tooltip on hover/focus because when you click the avatar, you see the full status in the menu anyway.

@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 282bec1 to c896ddc Compare October 13, 2021 11:03
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

Tooltip seems to not live update? Bildschirmfoto von 2021-10-13 12-55-10

Should subscribe to: https://github.com/nextcloud/spreed/blob/98d679964a2352a4fa455c514944273a2b14b778/src/components/RightSidebar/Participants/CurrentParticipants/CurrentParticipants.vue#L89-L95

I just removed the tooltip completely, since as the comment from Jan point out we get the same info when clicking on the menu :)

@ChristophWurst

This comment has been minimized.

Sign in to view
@CarlSchwan

This comment has been minimized.

Sign in to view
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from c896ddc to 58e450e Compare October 13, 2021 11:25
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 3e53b9d to 0e0f2a0 Compare October 13, 2021 11:33
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from c222467 to 0e0f2a0 Compare October 13, 2021 11:38
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch 3 times, most recently from a1d5a0e to fcf50ac Compare October 13, 2021 12:35
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

nickvergessen
nickvergessen reviewed Oct 13, 2021
View reviewed changes
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 8d0890c to 48a3a95 Compare October 13, 2021 15:42
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

@CarlSchwan CarlSchwan added the 4. to release Ready to be released and/or waiting for tests to finish label Oct 13, 2021
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 48a3a95 to 6510477 Compare October 13, 2021 16:13
@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 13, 2021

/compile amend /

PVince81
PVince81 requested changes Oct 13, 2021
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

some concerns about escaping in the PHP template

@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 749a3ca to a1522f1 Compare October 13, 2021 18:47
@CarlSchwan CarlSchwan requested a review from PVince81 October 13, 2021 19:38
nickvergessen
nickvergessen reviewed Oct 13, 2021
View reviewed changes
PVince81
PVince81 approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Code looks good 👍

@skjnldsv
Copy link
Member

skjnldsv commented Oct 15, 2021

Tests fails

@CarlSchwan
Copy link
Member Author

CarlSchwan commented Oct 15, 2021

Test failure is not unrelated: ldap server disconnected. I'm going to squash the commits and then merge this with admin right (ci is really too slow currently)

@CarlSchwan
Add current status in navbar
134167e 
This ports the code to vue and if the user_status app is disabled this
goes back to the previous behavior.

The Avatar vue components detects changes of the status automatically.

Signed-off-by: Carl Schwan <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
@CarlSchwan CarlSchwan force-pushed the work/status-navbar-vue branch from 370c188 to 134167e Compare October 15, 2021 18:58
@CarlSchwan CarlSchwan merged commit ff67ada into master Oct 15, 2021
@CarlSchwan CarlSchwan deleted the work/status-navbar-vue branch October 15, 2021 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PVince81 PVince81 PVince81 approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

@artonge artonge Awaiting requested review from artonge

@szaimen szaimen Awaiting requested review from szaimen

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@CarlSchwan @nickvergessen @ChristophWurst @skjnldsv @PVince81 @jancborchardt