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

Searchfield: Listview-specific fixes for search close Button #5102

Merged
merged 10 commits into from
Apr 19, 2021

Conversation

rob2d
Copy link
Contributor

@rob2d rob2d commented Apr 16, 2021

Explain the details for making this change. What existing problem does the pull request solve?

Fixes listview-specific alignment issues

Related github/jira issue (required):

Related to #5090 #5074 but does not close these. Pushing as separate PR to isolate testing cases/code as there are many permutations of close button stylings.

Steps necessary to review your pull request (required):

-visit http://localhost:4000/components/swaplist/example-search.html and verify close button is aligned
image

  • also do the same for the same URLs above with ?theme=classic

Included in this Pull Request:

  • An e2e or functional test for the bug or feature.
  • A note to the change log.

@rob2d rob2d requested a review from a team as a code owner April 16, 2021 20:23
tmcconechy
tmcconechy previously approved these changes Apr 16, 2021
Copy link
Member

@tmcconechy tmcconechy left a comment

Choose a reason for hiding this comment

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

Looks good - im not sure how you want to fix all these similar issues. But it could be a case where you want to do them all in one PR. Because one could break another ect...But do what makes sense thats just a thought.

I was looking at the "double border" issue and half expecting that to be fixed here but if its later thats fine. If you tackle that it seems like you can change this rule

.listview-search .searchfield-wrapper {
  height: 38px;
}

@rob2d rob2d changed the title searchfield: listview-specific cases fixes for search close button al… Searchfield: Listview-Specific Cases Fixes for Search Close Button Apr 17, 2021
@rob2d rob2d changed the title Searchfield: Listview-Specific Cases Fixes for Search Close Button Searchfield: Listview-specific fixes for search close Button Apr 17, 2021
EdwardCoyle
EdwardCoyle previously approved these changes Apr 19, 2021
@rob2d rob2d dismissed stale reviews from EdwardCoyle and tmcconechy via b7205de April 19, 2021 14:13
@rob2d
Copy link
Contributor Author

rob2d commented Apr 19, 2021

@tmcconechy found that we can target the input to just be 100% between both themes so just tweaked the solution a tiny bit (and also good catch, thanks).

@rob2d
Copy link
Contributor Author

rob2d commented Apr 19, 2021

Iterated/rewrote one of the more-flakey e2e datagrid tests that was failing on this branch... and that was at least passing locally consistently. Hopefully it does not trigger other cases 🤞

@tmcconechy tmcconechy merged commit 695b640 into main Apr 19, 2021
@tmcconechy tmcconechy deleted the 5090-tabs-and-listview-searchfield-close-fix branch April 19, 2021 19:36
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