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

Split mx_AppPermissionWarning on _AppsDrawer.pcss into two classes #10824

Merged
merged 9 commits into from
May 16, 2023
Merged

Split mx_AppPermissionWarning on _AppsDrawer.pcss into two classes #10824

merged 9 commits into from
May 16, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented May 8, 2023 •
edited
Loading

For element-hq/element-web#25269

This PR intends to split mx_AppPermissionWarning into mx_AppPermission and mx_AppWarning.

Most of the styles of mx_AppPermissionWarning are required for AppPermisson except one block, so this PR suggests to split the style rules to create a CSS file for the component.

type: task

Signed-off-by: Suguru Hirahara [email protected]

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 May 8, 2023
@luixxiul luixxiul marked this pull request as ready for review May 9, 2023 09:08
@luixxiul luixxiul requested a review from a team as a code owner May 9, 2023 09:08
@luixxiul luixxiul requested review from andybalaam and t3chguy May 9, 2023 09:08
luixxiul
luixxiul commented May 9, 2023
View reviewed changes
res/css/components/views/elements/_AppPermission.pcss
*/

.mx_AppPermission {
> div {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mx_AppPermissionWarning_row has been added to every div so I guess the type selector should work better.

andybalaam
andybalaam approved these changes May 9, 2023
View reviewed changes
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

@andybalaam andybalaam enabled auto-merge May 9, 2023 12:37
@andybalaam andybalaam added the X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue label May 9, 2023
auto-merge was automatically disabled May 11, 2023 04:04

Head branch was pushed to by a user without write access

@luixxiul luixxiul requested a review from andybalaam May 11, 2023 04:05
@andybalaam andybalaam added this pull request to the merge queue May 11, 2023
@luixxiul
Copy link
Contributor Author

https://percy.io/dfde73bd/matrix-react-sdk/builds/27304995/changed/1519329818?browser=firefox&browser_ids=36%2C38%2C39&subcategories=unreviewed%2Cchanges_requested&viewLayout=side-by-side&viewMode=new&width=486&widths=486%2C1024%2C1920

Snapshots User settings tab* are based on #10697.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 11, 2023
@andybalaam
Copy link
Member

no artifiacts found again. re-kicking, and considering moving working on that flakiness up my personal TODO list 😠

@andybalaam andybalaam added this pull request to the merge queue May 11, 2023
@andybalaam andybalaam mentioned this pull request May 11, 2023
Closed
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 11, 2023
@andybalaam
Copy link
Member

One more try

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 11, 2023
@luixxiul luixxiul marked this pull request as draft May 11, 2023 16:04
@luixxiul
Copy link
Contributor Author

I noticed just now that merging #10775 which have been created before this PR would create conflicts. I'll make this PR ready for review again, once the conflicts are resolved…

@luixxiul luixxiul marked this pull request as ready for review May 11, 2023 19:14
@luixxiul luixxiul requested a review from andybalaam May 11, 2023 19:26
@luixxiul
Copy link
Contributor Author

The conflicts were fixed.

@luixxiul
Copy link
Contributor Author

I think Percy check on the queue should return an error in the same way as #10836 (comment).

@andybalaam andybalaam added this pull request to the merge queue May 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 12, 2023
@andybalaam andybalaam added this pull request to the merge queue May 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 12, 2023
@t3chguy t3chguy removed their request for review May 12, 2023 11:20
@andybalaam andybalaam added this pull request to the merge queue May 12, 2023
@andybalaam andybalaam removed this pull request from the merge queue due to a manual request May 12, 2023
@andybalaam
Copy link
Member

andybalaam commented May 12, 2023 •
edited
Loading

"Style Lint" job failed with what looks like a momentary network outage at github, so trying again.

@andybalaam andybalaam added this pull request to the merge queue May 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 12, 2023
@luixxiul luixxiul marked this pull request as draft May 12, 2023 17:45
@luixxiul luixxiul marked this pull request as ready for review May 12, 2023 18:05
@luixxiul

This comment was marked as outdated.

Sign in to view
@andybalaam
Copy link
Member

Force merging since it contains new snapshots (which pass).

@andybalaam andybalaam merged commit 2eedfbf into matrix-org:develop May 16, 2023
@luixxiul luixxiul deleted the AppPermission branch May 16, 2023 10:20
@luixxiul luixxiul mentioned this pull request May 16, 2023
Merged
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@andybalaam andybalaam Awaiting requested review from andybalaam

Assignees
No one assigned
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@luixxiul @andybalaam