[Code] Search results are fixed to top of page #45182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rylnd
added
Team:Code
release_note:skip
|
Pinging @elastic/code |
rylnd
changed the title
mw-ding
approved these changes
Sep 9, 2019
There was a problem hiding this comment.
LGTM. @daveyholler can you take a look on the design's end?
💚 Build Succeeded |
|
@daveyholler @WangQianliang since this is my first PR to kibana, any and all comments are greatly appreciated. |
daveyholler
requested changes
Sep 9, 2019
| @@ -171,12 +171,12 @@ | |||
| white-space: nowrap; | |||
| } | |||
|
|
|||
| .codeSearch__suggestion-text-container { | |||
There was a problem hiding this comment.
I'm guessing this code has been around for a while, but we should be using snake case in these instances rather than hyphens: codeSearch__suggestionTextContainer
There was a problem hiding this comment.
Yep, I noticed some inconsistencies with BEM classes here; I'm happy to change that. Should I leave the existing classes for now?
rylnd
force-pushed
the
fixed-position-main-searchbar
branch
from
6cdeb8a to
ba5f91a
Compare
💔 Build Failed |
|
Please merge master |
💚 Build Succeeded |
By allowing overflow on the main content, we implicitly fix the sibling search bar in position. elastic/code#1606
This gives the parent element a width, so that our text-overflow rules apply to these elements. See https://bugzilla.mozilla.org/show_bug.cgi?id=1086218#c4 for details on the issue.
If both the icon and the text are allowed to grow/shrink as needed, flexbox will grow truncated text based on its original width, leading to subtle sub-pixel alignment issues with other rows. By fixing the icon to a specific width, we can ensure that the suggestion text does not grow too big.
We don't need to flex these elements currently as they contain either no children or inline elements only.
Without this directive on the parent, it had no width or flex growth and didn't respect the parent width. This allows it to take on the correct flexed width, and thus take on the overflow rules.
We have an overriding align-items declaration immediately after this one.
💚 Build Succeeded |
rylnd
force-pushed
the
fixed-position-main-searchbar
branch
from
62bd14f to
9d25daf
Compare
daveyholler
approved these changes
Sep 10, 2019
💚 Build Succeeded |
WangQianliang
approved these changes
Sep 11, 2019
rylnd
added a commit
to rylnd/kibana
that referenced
this pull request
Sep 11, 2019
* Fix main search bar to top of screen By allowing overflow on the main content, we implicitly fix the sibling search bar in position. elastic/code#1606 * Allow search suggestion text to be truncated This gives the parent element a width, so that our text-overflow rules apply to these elements. See https://bugzilla.mozilla.org/show_bug.cgi?id=1086218#c4 for details on the issue. * Fix alignment issue with overflowed search suggestions If both the icon and the text are allowed to grow/shrink as needed, flexbox will grow truncated text based on its original width, leading to subtle sub-pixel alignment issues with other rows. By fixing the icon to a specific width, we can ensure that the suggestion text does not grow too big. * Remove unneeded CSS We don't need to flex these elements currently as they contain either no children or inline elements only. * style: prefer camelCase element names over hyphens * Fix search result suggestion overflow on IE Without this directive on the parent, it had no width or flex growth and didn't respect the parent width. This allows it to take on the correct flexed width, and thus take on the overflow rules. * Remove unused CSS rule We have an overriding align-items declaration immediately after this one.
rylnd
added a commit
to rylnd/kibana
that referenced
this pull request
Sep 11, 2019
* Fix main search bar to top of screen By allowing overflow on the main content, we implicitly fix the sibling search bar in position. elastic/code#1606 * Allow search suggestion text to be truncated This gives the parent element a width, so that our text-overflow rules apply to these elements. See https://bugzilla.mozilla.org/show_bug.cgi?id=1086218#c4 for details on the issue. * Fix alignment issue with overflowed search suggestions If both the icon and the text are allowed to grow/shrink as needed, flexbox will grow truncated text based on its original width, leading to subtle sub-pixel alignment issues with other rows. By fixing the icon to a specific width, we can ensure that the suggestion text does not grow too big. * Remove unneeded CSS We don't need to flex these elements currently as they contain either no children or inline elements only. * style: prefer camelCase element names over hyphens * Fix search result suggestion overflow on IE Without this directive on the parent, it had no width or flex growth and didn't respect the parent width. This allows it to take on the correct flexed width, and thus take on the overflow rules. * Remove unused CSS rule We have an overriding align-items declaration immediately after this one.
rylnd
added a commit
that referenced
this pull request
Sep 11, 2019
* Fix main search bar to top of screen By allowing overflow on the main content, we implicitly fix the sibling search bar in position. elastic/code#1606 * Allow search suggestion text to be truncated This gives the parent element a width, so that our text-overflow rules apply to these elements. See https://bugzilla.mozilla.org/show_bug.cgi?id=1086218#c4 for details on the issue. * Fix alignment issue with overflowed search suggestions If both the icon and the text are allowed to grow/shrink as needed, flexbox will grow truncated text based on its original width, leading to subtle sub-pixel alignment issues with other rows. By fixing the icon to a specific width, we can ensure that the suggestion text does not grow too big. * Remove unneeded CSS We don't need to flex these elements currently as they contain either no children or inline elements only. * style: prefer camelCase element names over hyphens * Fix search result suggestion overflow on IE Without this directive on the parent, it had no width or flex growth and didn't respect the parent width. This allows it to take on the correct flexed width, and thus take on the overflow rules. * Remove unused CSS rule We have an overriding align-items declaration immediately after this one.
rylnd
added a commit
that referenced
this pull request
Sep 11, 2019
* Fix main search bar to top of screen By allowing overflow on the main content, we implicitly fix the sibling search bar in position. elastic/code#1606 * Allow search suggestion text to be truncated This gives the parent element a width, so that our text-overflow rules apply to these elements. See https://bugzilla.mozilla.org/show_bug.cgi?id=1086218#c4 for details on the issue. * Fix alignment issue with overflowed search suggestions If both the icon and the text are allowed to grow/shrink as needed, flexbox will grow truncated text based on its original width, leading to subtle sub-pixel alignment issues with other rows. By fixing the icon to a specific width, we can ensure that the suggestion text does not grow too big. * Remove unneeded CSS We don't need to flex these elements currently as they contain either no children or inline elements only. * style: prefer camelCase element names over hyphens * Fix search result suggestion overflow on IE Without this directive on the parent, it had no width or flex growth and didn't respect the parent width. This allows it to take on the correct flexed width, and thus take on the overflow rules. * Remove unused CSS rule We have an overriding align-items declaration immediately after this one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
elastic/code#1606
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilitySince this is my first PR:
For maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately