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

FIX Treedropdown css #1166

Merged

Conversation

emteknetnz
Copy link
Member

Fixes #853

Copy link
Contributor

@bergice bergice left a comment

Choose a reason for hiding this comment

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

We should probably also fix the text colour while we're at it.

.is-open>.Select-control,
.Select-menu-outer {
.Select-arrow-zone {
vertical-align: initial;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to actually just move the arrow up by about 1 pixel instead of vertically centering it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure about this? See screenshot below

Copy link
Contributor

Choose a reason for hiding this comment

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

image

This is what it looks like for me.
Left is the current look.
Right is with vertical-align: initial; applied.

Looks to me like we don't need to do anything here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Have removed the vertical align

@emteknetnz emteknetnz assigned emteknetnz and unassigned emteknetnz Feb 2, 2021
@emteknetnz
Copy link
Member Author

emteknetnz commented Feb 2, 2021

@bergice

a) What's wrong with the text colour? I'm not sure what you're referring to?

b) Re the arrow alignment, testing in chrome, it seems fine?:

Before:
image

After:
image

@emteknetnz emteknetnz force-pushed the pulls/1.7/treedropdown-css branch from f791ef2 to 70d3342 Compare February 2, 2021 21:30
.is-open>.Select-control,
.Select-menu-outer {
.Select-arrow-zone {
vertical-align: initial;
Copy link
Contributor

Choose a reason for hiding this comment

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

image

This is what it looks like for me.
Left is the current look.
Right is with vertical-align: initial; applied.

Looks to me like we don't need to do anything here.

Comment on lines 108 to 113
.Select-control {
border-color: $gray-200;
box-shadow: none;
}

.Select.is-focused:not(.is-open) > .Select-control,
.is-open > .Select-control {
border-color: $brand-primary;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

image

Need to move box shadow to .Select.is-focused:not(.is-open) > .Select-control.

Copy link
Member Author

Choose a reason for hiding this comment

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

Have added box-shadow: none so that the box shadows doesn't show

The box shadows comes from react-select - the DSM linked in the parent issue has no box-shadows so this seems correct https://projects.invisionapp.com/dsm/silver-stripe/silver-stripe/asset/components/5acd939108f7520011a65e5c

@emteknetnz emteknetnz changed the base branch from 1.7 to 1.8 June 17, 2021 23:16
@emteknetnz emteknetnz force-pushed the pulls/1.7/treedropdown-css branch from 70d3342 to 13ad24b Compare June 17, 2021 23:26
@emteknetnz emteknetnz force-pushed the pulls/1.7/treedropdown-css branch from 13ad24b to 771cb5f Compare June 17, 2021 23:56
@bergice bergice merged commit bd8b138 into silverstripe:1.8 Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TreeDropdownField hover state text colour broken
2 participants