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

feat(ConversationIcon): highlight public conversations #11450

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Jan 25, 2024

☑️ Resolves

  • Conversation (NcListItem) now indicates, if conversation is public (available for guests by link)

  • Place is details above the unread counter

  • To discuss:

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🌞 Light theme 🌑 Dark theme
Default with --color-primary-element-text for active --color-text-maxcontrast for others
image image

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖌️ Design was reviewed, approved or inspired by the design team

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

I think we should avoid adding this information on the avatar @Antreesy
Let me think about this and will come up with an alternative proposal soon.

@marcoambrosini marcoambrosini self-assigned this Jan 25, 2024
@Antreesy

This comment was marked as resolved.

@Antreesy Antreesy marked this pull request as draft January 28, 2024 15:01
@Antreesy Antreesy force-pushed the feat/noid/highlight-public-conversations branch from 4536a7e to 39d68d1 Compare January 31, 2024 14:01
@Antreesy Antreesy marked this pull request as ready for review January 31, 2024 14:01
@Antreesy
Copy link
Contributor Author

Rebased and updated screenshots.
Don't think that group conversations require that 'multiple-account' icon, to not overwhelm the sidebar, but we could discuss that

Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Code-wise, it's fine ✅
We can keep the same icon for federated conversations or it should be changed for some reason ?

@marcoambrosini
Copy link
Member

I think it would be nice info @Antreesy , shouldn't overwhelm much

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the feat/noid/highlight-public-conversations branch from 39d68d1 to cb774ad Compare February 1, 2024 11:18
@Antreesy Antreesy enabled auto-merge February 1, 2024 11:19
@Antreesy Antreesy merged commit 3c4b214 into main Feb 1, 2024
35 checks passed
@Antreesy Antreesy deleted the feat/noid/highlight-public-conversations branch February 1, 2024 11:23
@marcoambrosini
Copy link
Member

@Antreesy could we use the same color as the title for those icons?

@Antreesy
Copy link
Contributor Author

Antreesy commented Feb 2, 2024

same color as the title

This would differ from the default color (which was picked for the component in the library), so we kept it as-is for now

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