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.

Sign up for GitHub

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

Jump to bottom

Changing background color to align with EUI color #54060

Merged
merged 2 commits into from
Jan 7, 2020
Merged

Changing background color to align with EUI color #54060

merged 2 commits into from
Jan 7, 2020

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Jan 6, 2020 •
edited
Loading

Summary

Applying euiPageBackgroundColor to application class to remove discrepancy between the EUI background and Kibana apps background.
Fixes: #53867
Some apps where I tested this:
Visualizations
Screenshot 2020-01-06 at 20 59 00
Dashboard:
Screenshot 2020-01-06 at 20 59 47
Maps:
Screenshot 2020-01-06 at 21 00 03
Home:
Screenshot 2020-01-06 at 20 59 30

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@majagrubic majagrubic requested a review from a team as a code owner January 6, 2020 21:07
@majagrubic majagrubic added v7.6.0 v8.0.0 release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jan 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@majagrubic majagrubic requested a review from a team January 6, 2020 21:08
cchaos
cchaos suggested changes Jan 6, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment •
edited
Loading

Choose a reason for hiding this comment

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

My main concern with this solution is that it seems to be overriding what really is a regression. Somewhere a CSS style is being output that changes the body background color to the wrong color.

Screen Shot 2020-01-06 at 17 46 54 PM

We need to find that particular style, it may be in JS, and change it to the correct color or really just removed entirely since the background color should be added via EUI.

image

@cchaos
Copy link
Contributor

cchaos commented Jan 6, 2020

@majagrubic Found the culprit:

kibana/src/core/server/rendering/views/styles.tsx

Line 31 in ecab207

const themeBackground = darkMode ? '#25262e' : '#f5f7fa';

@majagrubic majagrubic requested a review from a team as a code owner January 7, 2020 09:25
@majagrubic
Copy link
Contributor Author

Urgh, that's the problem of styling in JS, as you cannot reuse variables from sass.
Thanks @cchaos, great find 🙇‍♀

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@majagrubic majagrubic requested a review from cchaos January 7, 2020 15:53
cchaos
cchaos approved these changes Jan 7, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thanks!

@majagrubic majagrubic merged commit cca454f into elastic:master Jan 7, 2020
@majagrubic majagrubic deleted the background-color branch January 7, 2020 16:36
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Jan 7, 2020
Changing background color to align with EUI color (elastic#54060)
7dbc937 
* Changing background color to align with EUI color

* Removing background-color from theme
@majagrubic majagrubic mentioned this pull request Jan 7, 2020
Merged
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 7, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/conso…
792771e 
…le-saved-objects

* 'master' of github.com:elastic/kibana: (86 commits)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  [SIEM] Enable eslint react/no-children-prop (elastic#53985)
  [DOCS][Reporting] Adds info about using a custom reporting index (elastic#54052)
  Changing background color to align with EUI color (elastic#54060)
  Fix linting issues (elastic#54068)
  NP Migration: Move doc views registry and existing doc views into discover plugin (elastic#53465)
  [ML] DF Analytics job creation: Add 'excludes' input field to form (elastic#53856)
  EMT-issue-65: add endpoint list api (elastic#53861)
  [SIEM] Fix doubled drag handles in Timeline (elastic#52679)
  Functional tests: refactor visualize_page (elastic#53845)
  [Dashboard] Redesign empty screen in readonly mode (elastic#54073)
  [ML] Support search for partitions on Single Metric Viewer (elastic#53879)
  update apm index pattern (elastic#54095)
  Add data ui for envoyproxy Metricbeat Module (elastic#53476)
  [ML] persist the brush when expanded to full width (elastic#54020)
  Skip failing test (elastic#54100)
  [APM] Show errors on the timeline instead of under the transaction (elastic#53756)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/public/np_ready/application/contexts/services_context.tsx
#	src/legacy/core_plugins/console/public/np_ready/application/index.tsx
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 7, 2020
@jloleysens
Merge branch 'feature/console-saved-objects' of github.com:jloleysens…
760d29f 
…/kibana into feature/console-saved-objects

* 'feature/console-saved-objects' of github.com:jloleysens/kibana: (86 commits)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  [SIEM] Enable eslint react/no-children-prop (elastic#53985)
  [DOCS][Reporting] Adds info about using a custom reporting index (elastic#54052)
  Changing background color to align with EUI color (elastic#54060)
  Fix linting issues (elastic#54068)
  NP Migration: Move doc views registry and existing doc views into discover plugin (elastic#53465)
  [ML] DF Analytics job creation: Add 'excludes' input field to form (elastic#53856)
  EMT-issue-65: add endpoint list api (elastic#53861)
  [SIEM] Fix doubled drag handles in Timeline (elastic#52679)
  Functional tests: refactor visualize_page (elastic#53845)
  [Dashboard] Redesign empty screen in readonly mode (elastic#54073)
  [ML] Support search for partitions on Single Metric Viewer (elastic#53879)
  update apm index pattern (elastic#54095)
  Add data ui for envoyproxy Metricbeat Module (elastic#53476)
  [ML] persist the brush when expanded to full width (elastic#54020)
  Skip failing test (elastic#54100)
  [APM] Show errors on the timeline instead of under the transaction (elastic#53756)
  ...
majagrubic pushed a commit that referenced this pull request Jan 8, 2020
Changing background color to align with EUI color (#54060) (#54148)
6113a61 
* Changing background color to align with EUI color

* Removing background-color from theme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pgayvallet pgayvallet pgayvallet approved these changes

@cchaos cchaos cchaos approved these changes

Assignees
No one assigned
Labels
release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.6.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Two different background colors in some apps
5 participants
@majagrubic @elasticmachine @cchaos @kibanamachine @pgayvallet