Skip to content

Conversation

@marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Jan 7, 2021

Screenshot from 2021-01-07 15-44-45

Signed-off-by: Marco Ambrosini [email protected]

@marcoambrosini marcoambrosini force-pushed the bugfix/noid/move-conversation-settings-to-conversation-settings branch 2 times, most recently from 6c38773 to 108fff7 Compare January 7, 2021 14:38
@marcoambrosini marcoambrosini marked this pull request as ready for review January 7, 2021 14:45
@marcoambrosini marcoambrosini self-assigned this Jan 7, 2021
@marcoambrosini marcoambrosini force-pushed the bugfix/noid/move-conversation-settings-to-conversation-settings branch from 108fff7 to f8d1f66 Compare January 7, 2021 14:52
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Nice!

I don't know why the action button is not correctly sized.
plop

Also, Matterbridge log is displayed in a modal on top of the settings modal. It's not shocking but what do you think about that?

Except that, looking good.

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

See comment from Julien

@marcoambrosini marcoambrosini force-pushed the bugfix/noid/move-conversation-settings-to-conversation-settings branch from f8d1f66 to f3dcec5 Compare January 12, 2021 06:44
Signed-off-by: Marco Ambrosini <[email protected]>
@marcoambrosini
Copy link
Member Author

Fixed the actions button hover feedback height and adjusted margins a bit

Also, Matterbridge log is displayed in a modal on top of the settings modal. It's not shocking but what do you think about that?

While I agree that it's not very elegant, I think that overall this is an improvement, especially the fact that we're moving every conversation related setting in one place as opposed to having them scattered around the UI. So I would say let's move forward with this :)

@nickvergessen
Copy link
Member

Peek 2021-01-12 13-21

Can the hovers have the same color?

@nickvergessen
Copy link
Member

Okay it's a bug in the vue lib...

@nickvergessen
Copy link
Member

Reported at nextcloud-libraries/nextcloud-vue#1665

@nickvergessen nickvergessen merged commit e6b75d8 into master Jan 12, 2021
@nickvergessen nickvergessen deleted the bugfix/noid/move-conversation-settings-to-conversation-settings branch January 12, 2021 12:46
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.

4 participants