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

Add current status in navbar #29210

Merged
merged 1 commit into from
Oct 15, 2021
Merged

Add current status in navbar #29210

merged 1 commit into from
Oct 15, 2021

Conversation

CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Oct 13, 2021

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 carl@carlschwan.eu

@CarlSchwan CarlSchwan requested review from jancborchardt, artonge, a team, PVince81 and szaimen and removed request for a team October 13, 2021 10:42
@CarlSchwan
Copy link
Member Author

/compile amend /

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
Copy link
Member Author

/compile amend /

@CarlSchwan
Copy link
Member Author

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.

@CarlSchwan

This comment has been minimized.

@CarlSchwan
Copy link
Member Author

/compile amend /

@CarlSchwan
Copy link
Member Author

/compile amend /

@CarlSchwan
Copy link
Member Author

/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
Copy link
Member Author

/compile amend /

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

core/templates/layout.user.php Outdated Show resolved Hide resolved
lib/private/TemplateLayout.php Show resolved Hide resolved
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

Tests fails

@CarlSchwan
Copy link
Member Author

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)

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 <carl@carlschwan.eu>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@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
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants