Skip to content

Use outline icons where it makes sense - #288

Merged
julien-nc merged 4 commits into
mainfrom
enh/noid/icons
Jun 30, 2025
Merged

Use outline icons where it makes sense#288
julien-nc merged 4 commits into
mainfrom
enh/noid/icons

Conversation

@julien-nc

@julien-ncjulien-nc commented Jun 27, 2025

Copy link
Copy Markdown
Member

And replace custom icons by material ones.

@julien-ncjulien-nc added enhancement New feature or request 3. to review labels Jun 27, 2025
@janepie

janepie commented Jun 27, 2025

Copy link
Copy Markdown
Member

SendIcon to SendOutlinedIcon?
And maybe InformationBox to InformationBoxOutline but it's really not pretty

@edward-ly

edward-ly commented Jun 27, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

also ShareVariantIcon to ShareVariantOutlineIcon, maybe

@julien-nc

julien-nc commented Jun 27, 2025

Copy link
Copy Markdown
MemberAuthor

SendOutlinedIcon looks bad, the normal one is not so "thick". I think that's ok.
ShareVariantOutlineIcon does not look good either IMO.

@edward-ly

edward-ly commented Jun 27, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

That's fine. How about:

  • PencilIcon to PencilOutlineIcon
  • FileDocumentIcon to FileDocumentOutlineIcon

@julien-nc

julien-nc commented Jun 27, 2025

Copy link
Copy Markdown
MemberAuthor

@edward-ly Yep, done, thanks.

Base automatically changed from enh/noid/vue3 to mainJune 30, 2025 10:41
kyteinsky
kyteinsky reviewed Jun 30, 2025
View reviewed changes

@kyteinskykyteinsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not really related to vue 3 migration but it could be a good place to fix this. It happens when the text contains large texts that cannot be wrapped on spaces like long URLs. The browser window has been shrunk horizontally too.

image

These seem like vue 3 regression/prop change.

  1. User status is disabled so should not be displayed for "admin". The new prop is hideStatus .
  2. Assistant icon is not centered

image

@julien-nc
feat: use outline icons where it makes sense, replace custom icons by…
b3b8484
… material ones
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc
fix(chat): give proper height to the custom assistant avatar icon in …
0bad298
…the chat
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc
fix(translate): give half of the width to each column
783c6a9
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc
julien-nc requested a review from kyteinskyJune 30, 2025 13:42
@julien-nc

julien-nc commented Jun 30, 2025

Copy link
Copy Markdown
MemberAuthor

@kyteinsky Thanks. I think both those issues are now fixed (with the last 2 commits).

@julien-nc
fix(chat): hide avatar user status
9bb0392
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
kyteinsky
kyteinsky approved these changes Jun 30, 2025
View reviewed changes

@kyteinskykyteinsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✨

@julien-nc
julien-nc merged commit 6d292e5 into mainJun 30, 2025
@julien-nc
julien-nc deleted the enh/noid/icons branch June 30, 2025 15:31
@janepiejanepie mentioned this pull request Aug 7, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyteinskykyteinskykyteinsky approved these changes
@edward-lyedward-lyAwaiting requested review from edward-ly
@janepiejanepieAwaiting requested review from janepie

Assignees

No one assigned

Labels

3. to reviewenhancementNew feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@julien-nc@janepie@edward-ly@kyteinsky