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

CSS for new TagPanel #5723

Merged
merged 2 commits into from
Nov 29, 2017
Merged

CSS for new TagPanel #5723

merged 2 commits into from
Nov 29, 2017

Conversation

lukebarnard1
Copy link
Contributor

@lukebarnard1 lukebarnard1 commented Nov 29, 2017

@@ -33,14 +33,13 @@ limitations under the License.
font-weight: 600;
font-size: 12px;
width: 203px; /* padding + width = LHS Panel width */
height: 17px; /* padding + height = 29px, same as mx_RoomSubList_stickyContainer */
height: 19px; /* padding + height = 29px, same as mx_RoomSubList_stickyContainer */
Copy link
Member

@dbkr dbkr Nov 29, 2017

Choose a reason for hiding this comment

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

You're replacing the 2px border with an extra 2px of height? What's the reason for doing so? And could we update the comment too, otherwise it becomes a lie since 19 + (6 * 2) != 29

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Nov 29, 2017

Choose a reason for hiding this comment

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

Yep, this seemed the easiest option, without the border or height there would be gaps between the "stuck" headers. Increasing the height is one option, another would have been leaving the border and colouring it the same as the background but that seemed a bit pointless.

I shall comment as such. mx_RoomSubList_stickyContainer doesn't exist so I'll probably just remove the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realised it should have been
/* height + padding = 31px = mx_RoomSubList_label height */

Copy link
Member

Choose a reason for hiding this comment

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

ok, yeah, I think that makes sense!

@lukebarnard1
Copy link
Contributor Author

Tests have flaked, surprise surprise

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Nov 29, 2017
@lukebarnard1 lukebarnard1 merged commit 123ad19 into develop Nov 29, 2017
@t3chguy t3chguy deleted the luke/feature-tag-panel branch May 12, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants