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

Add some style improvements to autocompletions #4456

Merged
merged 1 commit into from
Jun 28, 2017

Conversation

lukebarnard1
Copy link
Contributor

 - Fix #2230 by adding text-overflow: ellipsis to pill spans
 - Add padding to pills
 - Make sure to only apply horizontal margin of pill children at one level of the DOM tree
@lukebarnard1 lukebarnard1 requested review from aviraldg and removed request for aviraldg June 28, 2017 15:38
@lukebarnard1
Copy link
Contributor Author

Whoops, sorry @aviraldg! I butter-fingered there.

display: flex;
user-select: none;
cursor: pointer;
align-items: center;
color: $primary-fg-color;
}

.mx_Autocomplete_Completion_pill * {
.mx_Autocomplete_Completion_pill > * {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I slightly question whether being in an autocomplete pill implies things will want this CSS, or whether it would be better to put this on the children directly, but if this works then fine.

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 think it was the original intention. I think it's only worth changing if there's a child that doesn't want a margin, which for the time being there isn't 😇

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

otherwise lgtm

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr Jun 28, 2017
@lukebarnard1
Copy link
Contributor Author

the tests are failing for some totally unrelated reason... I'll try rerunning them

@lukebarnard1
Copy link
Contributor Author

There seems to be a persistent

    ^
Error: Cannot find module 'boom'

@lukebarnard1
Copy link
Contributor Author

lukebarnard1 commented Jun 28, 2017

I'm merging this because it can't have broken the tests 😇

@lukebarnard1 lukebarnard1 reopened this Jun 28, 2017
@lukebarnard1 lukebarnard1 merged commit 515f8de into develop Jun 28, 2017
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