-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Account for presence of top banner when EuiDataGrid and Canvas are full screen #111038
Merged
MichaelMarcialis
merged 14 commits into
elastic:master
from
MichaelMarcialis:109554/top-banner-full-screen-fixes
Sep 13, 2021
Merged
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b56f13a
account for banners when data grid is full screen
MichaelMarcialis b23599a
account for banner when canvas is full screen
MichaelMarcialis 710fdaf
Merge branch 'master' of github.com:elastic/kibana into 109554/top-ba…
MichaelMarcialis 860ea58
change height per feedback
MichaelMarcialis aa70475
Merge branch 'master' of github.com:elastic/kibana into 109554/top-ba…
MichaelMarcialis c0b0206
add withKibana
MichaelMarcialis 63a7df2
Merge branch 'master' of github.com:elastic/kibana into 109554/top-ba…
MichaelMarcialis 0efa8e4
rm withKibana; move vars out of Fullscreen
MichaelMarcialis 554aafe
Use hasHeaderBanner$
cqliu1 734c1f5
Merge pull request #2 from cqliu1/pr/111038
128872d
add banner height var comments
MichaelMarcialis b7abcb1
Merge branch 'master' of github.com:elastic/kibana into 109554/top-ba…
MichaelMarcialis 7b0e729
fix ts error
MichaelMarcialis 5e21b25
Merge branch 'master' into 109554/top-banner-full-screen-fixes
kibanamachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: This is number can easily get out sync between the JS and Sass versions. I'd highly suggest moving it a constant that lives within the actual banner service and imported by Canvas. Then in both places (JS & CSS), add a comment that mentions where it's counterpart lives. Something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pgayvallet: Thoughts on Caroline's suggestion above to move the banner height to live in the banner service? Sounds good to me, but would likely need your guidance on how best to implement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It definitely makes sense, but I think this can be done by my team as a follow-up (if that's fine with you). I created #111688
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. In the mean time, I'll add those suggested comments in the current variable locations.