Skip to content

Conversation description - front-end #4546

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.

Sign up for GitHub

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

Jump to bottom
Merged
nickvergessen merged 1 commit into from
Dec 18, 2020
Merged

Conversation description - front-end #4546

nickvergessen merged 1 commit into from
Dec 18, 2020

Conversation

@marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Nov 6, 2020 •
edited by nickvergessen
Loading

@marcoambrosini marcoambrosini changed the title temp Conversation description - front-end Nov 6, 2020
@marcoambrosini marcoambrosini force-pushed the feature/3432/conversation-description-frontend branch 3 times, most recently from bed2cfc to e3cd268 Compare November 11, 2020 12:41
@marcoambrosini marcoambrosini self-assigned this Nov 11, 2020
@marcoambrosini marcoambrosini added this to the 💚 Next Major (21) milestone Nov 11, 2020
PVince81
PVince81 requested changes Nov 11, 2020
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.

See comments.

@PVince81

This comment has been minimized.

Sign in to view
@marcoambrosini

This comment has been minimized.

Sign in to view
skjnldsv
skjnldsv reviewed Nov 13, 2020
View reviewed changes
@marcoambrosini marcoambrosini force-pushed the feature/3432/conversation-description-frontend branch 7 times, most recently from 126fa3d to 8a5248b Compare November 20, 2020 17:30
@marcoambrosini marcoambrosini marked this pull request as ready for review November 20, 2020 17:32
@marcoambrosini marcoambrosini force-pushed the feature/3432/conversation-description-frontend branch 3 times, most recently from 6be006b to 0ecd745 Compare November 23, 2020 14:21
@marcoambrosini

This comment has been minimized.

Sign in to view
@nickvergessen nickvergessen marked this pull request as draft November 27, 2020 12:27
@nickvergessen nickvergessen mentioned this pull request Dec 18, 2020
Merged
@PVince81
Copy link
Member

PVince81 commented Dec 18, 2020

Proposals:

  • make the text grey ?

image

  • links should be clickable
  • if you put two words followed by a link, the shadow effect that hints at a longer description is not visible for some reason

@PVince81

This comment has been minimized.

Sign in to view
@PVince81

This comment has been minimized.

Sign in to view
@PVince81
Copy link
Member

PVince81 commented Dec 18, 2020

  • I'd personally prefer to have the pencil icon a bit lighter as it's not something we click often so doesn't need to be as prominent

@PVince81
Copy link
Member

PVince81 commented Dec 18, 2020

@PVince81
Copy link
Member

PVince81 commented Dec 18, 2020

  • might need to make the description tabbable to be able to expand it with the keyboard (probably something for later), or maybe expand it directly when tabbing on top of it

PVince81
PVince81 reviewed Dec 18, 2020
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 fine, see minor comments.
Also see my comments about the styling and issues.

@marcoambrosini marcoambrosini force-pushed the feature/3432/conversation-description-frontend branch 2 times, most recently from c3d5e85 to 43cd471 Compare December 18, 2020 10:58
@marcoambrosini marcoambrosini force-pushed the feature/3432/conversation-description-frontend branch from 43cd471 to 7785382 Compare December 18, 2020 11:03
@marcoambrosini
Copy link
Member Author

marcoambrosini commented Dec 18, 2020

Addressed the important things, readi to merge :)

PVince81
PVince81 approved these changes Dec 18, 2020
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.

👍

@nickvergessen
Copy link
Member

nickvergessen commented Dec 18, 2020

Something for a Fix up, but currently while you can Shift+enter for a new line, on submitting they are stripped out. I guess the multiline prop is missing?

@nickvergessen
Copy link
Member

nickvergessen commented Dec 18, 2020

But good enough for version 1

@nickvergessen nickvergessen merged commit 8c7d09b into master Dec 18, 2020
@nickvergessen nickvergessen deleted the feature/3432/conversation-description-frontend branch December 18, 2020 12:09
@marcoambrosini
Copy link
Member Author

marcoambrosini commented Dec 18, 2020

Something for a Fix up, but currently while you can Shift+enter for a new line, on submitting they are stripped out.

No, that's something I implemented with the intention of trimming new lines only at the beginning and end of the string, so the removal of newlines in within the text is actually a bug

@jakobroehrl
Copy link
Contributor

jakobroehrl commented Dec 20, 2020

Nice function 👍
Wouldn't it be handy to show the description on top in the chat view?
I fear the users doesn't see it.

@marcoambrosini
Copy link
Member Author

marcoambrosini commented Dec 20, 2020

Wouldn't it be handy to show the description on top in the chat view?

I think that it would be a good Idea. But as of now the thing that I miss the most there is the title of the conversation

@jakobroehrl
Copy link
Contributor

jakobroehrl commented Dec 20, 2020

That's right, should I add a new issue: "Add groupname and description to the chat view"?

@marcoambrosini
Copy link
Member Author

marcoambrosini commented Dec 20, 2020

Sure, thanks @jakobroehrl :)

@jakobroehrl jakobroehrl mentioned this pull request Dec 20, 2020
Closed
This was referenced Feb 19, 2021
Merged
Merged
skjnldsv
skjnldsv reviewed Apr 20, 2022
View reviewed changes
src/components/RightSidebar/Description/Description.vue
color: var(--color-primary);
font-weight: bold;
font-size: var(--default-font-size);
line-height: var(----default-line-height);
Copy link
Member

@skjnldsv skjnldsv Apr 20, 2022

Choose a reason for hiding this comment

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

Suggested change
line-height: var(----default-line-height);
line-height: var(--default-line-height);

@marcoambrosini 🙈

Copy link
Member

@nickvergessen nickvergessen Apr 20, 2022

Choose a reason for hiding this comment

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

Fix at #7185

Copy link
Member Author

@marcoambrosini marcoambrosini Apr 20, 2022

Choose a reason for hiding this comment

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

oops

@nickvergessen nickvergessen mentioned this pull request Apr 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv left review comments

@PVince81 PVince81 PVince81 approved these changes

@danxuliu danxuliu Awaiting requested review from danxuliu

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

Assignees

@marcoambrosini marcoambrosini

Labels

3. to review feature: frontend 🖌️ "Web UI" client

Projects

None yet

Milestone

v11.0.0-alpha.1

Development

Successfully merging this pull request may close these issues.

Ability to add description to conversation

6 participants

@marcoambrosini @PVince81 @nickvergessen @jakobroehrl @skjnldsv