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

update universal results borders #708

Merged
merged 3 commits into from
Apr 21, 2020
Merged

update universal results borders #708

merged 3 commits into from
Apr 21, 2020

Conversation

oshi97
Copy link
Contributor

@oshi97 oshi97 commented Apr 21, 2020

margin on top and bottom of yxt-Results-filters
was not receiving border-left and border-right
(because margin does not receive border in css)
causing gaps to appear

TEST=manual
check that vertical results on vertical search,
vertical results on universal searchh, accordion results
do not have gaps in the border, and otherwise appearance is unchanged

margin on top and bottom of yxt-Results-filters
was not receiving border-left and border-right
(because margin does not receive border in css)
causing gaps to appear

TEST=manual
check that vertical results on vertical search,
vertical results on universal searchh, accordion results
do not have gaps in the border, and otherwise appearance is unchanged
@oshi97
Copy link
Contributor Author

oshi97 commented Apr 21, 2020

I'm slightly worried about this breaking somebody's page layout, but the only other alternative I could see was removing all of the border-left and border-right for everything in vertical results when it is in a universal result and adding a border around the container. This might be safer though?

background-color: $results-filters-background;
border-top: $results-border;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be removed from titleBar and added here? If possible, it would be better to leave below the title I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a second commit showing the other possible solution for the double borders (which gives yxt-Results-filters border-bottom instead of giving it a border-top).

Prior to this commit yxt-Results-filters never has a border-top or border-bottom, so if yxt-Results-filters is missing borders are going to double up. I can't think of something better than just giving it either border-bottom or border-top and moving other borders around?

Copy link
Contributor Author

@oshi97 oshi97 Apr 21, 2020

Choose a reason for hiding this comment

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

I think I like the first commit more, if only because it keeps the

  & .yxt-Accordion-list	
  {	
    border-top: $border-legacy;	
  }

they asked me to add ._. and it's slightly shorter

@oshi97 oshi97 merged commit ee88a29 into v0.13.3 Apr 21, 2020
@oshi97 oshi97 deleted the universal-results-borders branch April 21, 2020 18:18
tmeyer2115 pushed a commit that referenced this pull request Apr 24, 2020
* update universal results borders

margin on top and bottom of yxt-Results-filters
was not receiving border-left and border-right
(because margin does not receive border in css)
causing gaps to appear

Also fix double border

TEST=manual
check that vertical results on vertical search,
vertical results on universal searchh, accordion results
do not have gaps in the border, and otherwise appearance is unchanged
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.

None yet

3 participants