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

Introduce CSS custom properties to _TopUnreadMessagesBar.pcss #10796

Closed
wants to merge 9 commits into from
Closed

Introduce CSS custom properties to _TopUnreadMessagesBar.pcss #10796

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6115033
Nesting
luixxiul May 2, 2023
f5af56e
Run prettier
luixxiul May 2, 2023
34236d3
Group common style rules
luixxiul May 2, 2023
b0c701f
Separate declarations and their values by introducing custom properties
luixxiul May 4, 2023
f8b05a0
Nesting to fix the custom properties specificity
luixxiul May 4, 2023
f45351a
Run prettier
luixxiul May 4, 2023
65bfd99
Remove a redundant property
luixxiul May 5, 2023
7de8938
Set var(--width) and let it cascade for mx_TopUnreadMessagesBar_markA…
luixxiul May 5, 2023
7ace937
Remove duplicate properties
luixxiul May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 64 additions & 51 deletions res/css/views/rooms/_TopUnreadMessagesBar.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,60 +22,73 @@ limitations under the License.
top: 24px;
right: 24px;
width: 38px;
}

.mx_TopUnreadMessagesBar::after {
content: "";
position: absolute;
top: -8px;
left: 10.5px;
width: 4px;
height: 4px;
border-radius: 16px;
background-color: $secondary-accent-color;
border: 6px solid $accent;
pointer-events: none;
}
&::after {
content: "";
position: absolute;
top: -8px;
left: 10.5px;
width: 4px;
height: 4px;
border-radius: 16px;
background-color: $secondary-accent-color;
border: 6px solid $accent;
pointer-events: none;
}

.mx_TopUnreadMessagesBar_scrollUp {
height: 38px;
border-radius: 19px;
box-sizing: border-box;
background: $background;
border: 1.3px solid $muted-fg-color;
cursor: pointer;
}
/* Define common declarations. Their values are specified below. */
.mx_TopUnreadMessagesBar_scrollUp,
.mx_TopUnreadMessagesBar_markAsRead {
Comment on lines +40 to +41
Copy link
Member

Choose a reason for hiding this comment

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

what is the reason for nesting these where they were not before?

Copy link
Contributor Author

@luixxiul luixxiul May 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

To avoid repeating the same declarations, and to make the structure compact. Would you rather write like this?

    .mx_TopUnreadMessagesBar_scrollUp {
        background: var(--background);
        width: var(--width);
        height: var(--height);
        border: var(--border);
        border-radius: var(--borderRadius);

        --background: $background;
        --border: 1.3px solid $muted-fg-color;

        &::before {
            content: "";
            position: absolute;
            background: var(--background);
            width: var(--width);
            height: var(--height);
            mask-repeat: no-repeat;
            mask-position: center;
            mask-image: var(--maskImage);
            mask-size: var(--maskSize);

            --background: $muted-fg-color;
        }
    }

    .mx_TopUnreadMessagesBar_markAsRead {
        background: var(--background);
        width: var(--width);
        height: var(--height);
        border: var(--border);
        border-radius: var(--borderRadius);

        --background: $background;
        --border: 1.3px solid $muted-fg-color;

        &::before {
            content: "";
            position: absolute;
            background: var(--background);
            width: var(--width);
            height: var(--height);
            mask-repeat: no-repeat;
            mask-position: center;
            mask-image: var(--maskImage);
            mask-size: var(--maskSize);

            --background: $muted-fg-color;
        }
    }

It works, but it's not really efficient.

Copy link
Contributor Author

@luixxiul luixxiul May 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Also, it seems to me that there has not been a specific reason either that those selectors were not combined, checking a commit like ac9902e#diff-21b2361c0ea031da22f3680c5912c17ccc3d86c5bad901dd533f004cde3efa32 of #2345 and 07348a6#diff-21b2361c0ea031da22f3680c5912c17ccc3d86c5bad901dd533f004cde3efa32 of #4159.

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid I don't understand your answer. I think you are explaining why .mx_TopUnreadMessagesBar_scrollUp and .mx_TopUnreadMessagesBar_markAsRead are combined, but I didn't ask that. I asked why they are nested (inside .mx_TopUnreadMessagesBar).

background: var(--background);
width: var(--width);
height: var(--height);
border: var(--border);
border-radius: var(--borderRadius);
Copy link
Contributor Author

@luixxiul luixxiul May 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Please note that using names which seem general for properties does not matter here, because the custom properties cascade (this is one of the main differences between the custom property and the preprocessor's variable). Even if --height, for example, is specified somewhere outside of this file / the selector, its value is not respected, because the value is explicitly specified below for each selector (--height: 38px and --height: 18px) and overridden. Comments for each variable are also simply redundant, as those variables are limited to those selectors, and it is obvious by themselves how they work. It would be like adding a comment to a declaration display: inherit that the value for this declaration is inherited, which we would not do.


.mx_TopUnreadMessagesBar_scrollUp::before {
content: "";
position: absolute;
width: 36px;
height: 36px;
mask-image: url("$(res)/img/element-icons/message/chevron-up.svg");
mask-repeat: no-repeat;
mask-size: 20px;
mask-position: center;
background: $muted-fg-color;
}
--background: $background;
--border: 1.3px solid $muted-fg-color;

.mx_TopUnreadMessagesBar_markAsRead {
display: block;
width: 18px;
height: 18px;
background: $background;
border: 1.3px solid $muted-fg-color;
border-radius: 10px;
margin: 5px auto;
}
&::before {
content: "";
position: absolute;
background: var(--background);
width: var(--width);
height: var(--height);
mask-repeat: no-repeat;
mask-position: center;
mask-image: var(--maskImage);
mask-size: var(--maskSize);

.mx_TopUnreadMessagesBar_markAsRead::before {
content: "";
position: absolute;
width: 18px;
height: 18px;
mask-image: url("$(res)/img/cancel.svg");
mask-repeat: no-repeat;
mask-size: 10px;
mask-position: 4px 4px;
background: $muted-fg-color;
--background: $muted-fg-color;
}
}

.mx_TopUnreadMessagesBar_scrollUp {
--height: 38px;
--borderRadius: 19px;

box-sizing: border-box;
cursor: pointer;

&::before {
--width: 36px;
--height: 36px;
--maskImage: url("$(res)/img/element-icons/message/chevron-up.svg");
--maskSize: 20px;
}
}

.mx_TopUnreadMessagesBar_markAsRead {
--width: 18px;
--height: 18px;
--borderRadius: 10px;

display: block;
margin: 5px auto;

&::before {
--maskImage: url("$(res)/img/cancel.svg");
--maskSize: 10px;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--width: 18px and --height: 18px do not need to be repeated here, as they are inherited from mx_TopUnreadMessagesBar_markAsRead.

}
}