Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Deprecate mixin ThreadSummaryIcon#11023

Closed
luixxiul wants to merge 8 commits into
matrix-org:developfrom
luixxiul:Icon-thread
Closed

Deprecate mixin ThreadSummaryIcon#11023
luixxiul wants to merge 8 commits into
matrix-org:developfrom
luixxiul:Icon-thread

Conversation

@luixxiul
Copy link
Copy Markdown
Contributor

@luixxiul luixxiul commented Jun 1, 2023

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@github-actions github-actions Bot added Z-Community-PR Issue is solved by a community member's PR T-Task Refactoring, enabling or disabling functionality, other engineering tasks labels Jun 1, 2023
display: flex;
align-items: center;
column-gap: 8px;
margin-bottom: 8px;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please note $spacing variables have been deprecated: #10686

@luixxiul
Copy link
Copy Markdown
Contributor Author

luixxiul commented Jun 1, 2023

I'm struggling to increase the test coverage. Any pointers would be appreciated.

@weeman1337
Copy link
Copy Markdown
Contributor

I'm struggling to increase the test coverage. Any pointers would be appreciated.

Looking into the Sonar Cloud report adding a Jest test for EventTile could help here:
#11023 (comment)

Copy link
Copy Markdown
Contributor

@weeman1337 weeman1337 left a comment

Choose a reason for hiding this comment

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

Thank you @luixxiul . I think this is a very valuable contribution. 👍 Replacing these mask-based and pseudo-element icons with a component will help with the compound migration.

Can you have a look at the Jest tests? If you need assistance there let me know.

Comment thread res/css/_common.pcss Outdated
color: $username-variant8-color;
}

.mx_Icon--thread {
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 mandatory since the current icon stuff is temporary until Compound arrives™

You could add mx_Icon_18 and mx_Icon_secondary-content here.

@luixxiul

This comment was marked as off-topic.

@luixxiul
Copy link
Copy Markdown
Contributor Author

luixxiul commented Jun 19, 2023

Closing as I am not quite sure how I should proceed from here… @weeman1337 thanks anyway for the review.

@luixxiul luixxiul closed this Jun 19, 2023
@luixxiul luixxiul deleted the Icon-thread branch June 19, 2023 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

T-Task Refactoring, enabling or disabling functionality, other engineering tasks Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants