Skip to content

Commit

Permalink
Remove sass-vars-to-js-loader dependency (#6444)
Browse files Browse the repository at this point in the history
* Remove usage of sass-vars-to-js-loader to grab font weight

- copies existing `useJsonVars` usage in file

* Remove usage of sass-vars-to-js-loader for font weights

- import JSON file directly of `useJsonVars` since we're just grabbing font weight vars and theme color is irrelevant

* [optional] Refactor variable font range to use JS vars, not Sass

- this will have to be done anyway if/when we completely remove Sass etc., so it seems prescient to do now

- nothing end-user-facing should have changed as a result of this refactor

* Convert EUI charts themes to use JS theme vars instead of loading Sass vars

- some slightly opinionated changes in here - I've left TODOs on things I'm unsure about, will also elaborate more in github comments

* Remove `sass-vars-to-js-loader` dependency

* Ignore TS-error after merge

- interesting that it only started throwing after recent `.json.d.ts` additions - ah well

* Comments
- remove TODOs per designer feedback
  • Loading branch information
Cee authored Dec 2, 2022
1 parent 53449c1 commit fc30ca8
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 117 deletions.
2 changes: 1 addition & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# force unix-style LF (\n) line endings for source files
# this is necessary for prettier's linting and sass-vars-to-js-loader
# this is necessary for prettier's linting
*.js text eol=lf
*.ts text eol=lf
*.tsx text eol=lf
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@
"sass-lint": "^1.13.1",
"sass-lint-auto-fix": "^0.21.2",
"sass-loader": "^13.0.2",
"sass-vars-to-js-loader": "^2.1.1",
"shelljs": "^0.8.4",
"start-server-and-test": "^1.11.3",
"style-loader": "^3.3.1",
Expand Down
9 changes: 4 additions & 5 deletions src-docs/src/views/theme/typography/_typography_sass.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import React, { FunctionComponent, ReactNode } from 'react';

// @ts-ignore Importing from Sass file
import fonts from '!!sass-vars-to-js-loader?preserveKeys=true!../../../../../src/global_styling/variables/_font_weight.scss';

import { EuiCode, EuiBasicTable, EuiSpacer } from '../../../../../src';

// @ts-ignore Importing from JS
Expand Down Expand Up @@ -144,18 +141,20 @@ export const euiFontWeights = [
'euiFontWeightMedium',
'euiFontWeightSemiBold',
'euiFontWeightBold',
];
] as const;

export const FontWeightSass: FunctionComponent<ThemeRowType> = ({
description,
}) => {
const values = useJsonVars();

return (
<>
<ThemeExample
title={<code>$euiFontWeight[Weight]</code>}
description={description}
example={
<div style={{ fontWeight: fonts.euiFontWeightBold }}>
<div style={{ fontWeight: values.euiFontWeightBold }}>
I am proper bold
</div>
}
Expand Down
26 changes: 16 additions & 10 deletions src-docs/src/views/theme/typography/values.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, { useContext, useMemo, useState } from 'react';
import { Link } from 'react-router-dom';
import { css } from '@emotion/react';

// @ts-ignore Importing from Sass file
import fonts from '!!sass-vars-to-js-loader?preserveKeys=true!../../../../../src/global_styling/variables/_font_weight.scss';
// @ts-ignore Importing JSON
import themeVars from '../_json/eui_theme_light.json'; // Only grabbing font weight vars, so color mode is irrelevant

import { GuideSection } from '../../../components/guide_section/guide_section';

Expand Down Expand Up @@ -181,12 +182,17 @@ export default () => {
showValue
aria-label="Font weight"
showTicks
ticks={euiFontWeights.map(function (name) {
return {
label: name.split('euiFontWeight').pop(),
value: fonts[name],
};
})}
ticks={Object.entries(euiTheme.font.weight).map(
([name, value]) => ({
label: name,
value: Number(value),
})
)}
css={css`
.euiRangeTick {
text-transform: capitalize;
}
`}
/>
</>
}
Expand Down Expand Up @@ -241,8 +247,8 @@ const findJSFontWeight = ({
};

const findSassFontWeight = ({ fontWeight }: { fontWeight: number }) => {
const weight = Object.keys(fonts).find(
(key) => fonts[key] === Number(fontWeight)
const weight = euiFontWeights.find(
(key) => themeVars[key] === Number(fontWeight)
);
return `font-weight: ${weight ? `$${weight}` : fontWeight};`;
};
78 changes: 45 additions & 33 deletions src/themes/charts/themes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ import { PartialTheme, LineAnnotationSpec } from '@elastic/charts';

import { euiPaletteColorBlind } from '../../services/color/eui_palettes';
import { DEFAULT_VISUALIZATION_COLOR } from '../../services/color/visualization_colors';

// @ts-ignore typescript doesn't understand the webpack loader
import lightColors from '!!sass-vars-to-js-loader!../../themes/amsterdam/_colors_light.scss';
// @ts-ignore typescript doesn't understand the webpack loader
import darkColors from '!!sass-vars-to-js-loader!../../themes/amsterdam/_colors_dark.scss';
import { tint, shade } from '../../services/color';
import { buildTheme, getComputed } from '../../services/theme/utils';
import { EuiThemeAmsterdam } from '../../themes/amsterdam/theme';

const fontFamily = `'Inter', 'Inter UI', -apple-system, BlinkMacSystemFont,
'Segoe UI', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'`;
Expand All @@ -29,19 +27,19 @@ function createTheme(colors: any): EuiChartThemeType {
lineAnnotation: {
line: {
strokeWidth: 1,
stroke: colors.euiColorDarkShade.rgba,
stroke: colors.darkShade,
opacity: 1,
},
details: {
fontSize: 10,
fontFamily: fontFamily,
fill: colors.euiTextColor.rgba,
fill: colors.text,
padding: 0,
},
},
theme: {
background: {
color: colors.euiColorEmptyShade.rgba,
color: colors.emptyShade,
},
chartMargins: {
left: 0,
Expand All @@ -54,7 +52,7 @@ function createTheme(colors: any): EuiChartThemeType {
strokeWidth: 2,
},
point: {
fill: colors.euiColorEmptyShade.rgba,
fill: colors.emptyShade,
strokeWidth: 2,
radius: 3,
},
Expand All @@ -68,7 +66,7 @@ function createTheme(colors: any): EuiChartThemeType {
},
point: {
visible: false,
fill: colors.euiColorEmptyShade.rgba,
fill: colors.emptyShade,
strokeWidth: 2,
radius: 3,
},
Expand All @@ -94,40 +92,40 @@ function createTheme(colors: any): EuiChartThemeType {
axisTitle: {
fontSize: 12,
fontFamily: fontFamily,
fill: colors.euiTextColor.rgba,
fill: colors.text,
padding: {
inner: 10,
outer: 0,
},
},
axisLine: {
stroke: colors.euiColorChartLines.rgba,
stroke: colors.chartLines,
},
tickLabel: {
fontSize: 10,
fontFamily: fontFamily,
fill: colors.euiTextSubduedColor.rgba,
fill: colors.subduedText,
padding: {
outer: 8,
inner: 10,
},
},
tickLine: {
visible: false,
stroke: colors.euiColorChartLines.rgba,
stroke: colors.chartLines,
strokeWidth: 1,
},
gridLine: {
horizontal: {
visible: true,
stroke: colors.euiColorChartLines.rgba,
stroke: colors.chartLines,
strokeWidth: 1,
opacity: 1,
dash: [0, 0],
},
vertical: {
visible: true,
stroke: colors.euiColorChartLines.rgba,
stroke: colors.chartLines,
strokeWidth: 1,
opacity: 1,
dash: [4, 4],
Expand All @@ -140,48 +138,48 @@ function createTheme(colors: any): EuiChartThemeType {
},
crosshair: {
band: {
fill: colors.euiColorChartBand.rgba,
fill: colors.chartBand,
},
line: {
stroke: colors.euiColorDarkShade.rgba,
stroke: colors.darkShade,
strokeWidth: 1,
dash: [4, 4],
},
crossLine: {
stroke: colors.euiColorDarkShade.rgba,
stroke: colors.darkShade,
strokeWidth: 1,
dash: [4, 4],
},
},
goal: {
tickLabel: {
fontFamily: fontFamily,
fill: colors.euiTextSubduedColor.rgba,
fill: colors.subduedText,
},
majorLabel: {
fontFamily: fontFamily,
fill: colors.euiTextColor.rgba,
fill: colors.text,
},
minorLabel: {
fontFamily: fontFamily,
fill: colors.euiTextSubduedColor.rgba,
fill: colors.subduedText,
},
majorCenterLabel: {
fontFamily: fontFamily,
fill: colors.euiTextColor.rgba,
fill: colors.text,
},
minorCenterLabel: {
fontFamily: fontFamily,
fill: colors.euiTextSubduedColor.rgba,
fill: colors.subduedText,
},
targetLine: {
stroke: colors.euiColorDarkestShade.rgba,
stroke: colors.darkestShade,
},
tickLine: {
stroke: colors.euiColorMediumShade.rgba,
stroke: colors.mediumShade,
},
progressLine: {
stroke: colors.euiColorDarkestShade.rgba,
stroke: colors.darkestShade,
},
},
partition: {
Expand All @@ -196,21 +194,35 @@ function createTheme(colors: any): EuiChartThemeType {
linkLabel: {
maxCount: 5,
fontSize: 11,
textColor: colors.euiTextColor.rgba,
textColor: colors.text,
},
outerSizeRatio: 1,
circlePadding: 4,
sectorLineStroke: colors.euiColorEmptyShade.rgba,
sectorLineStroke: colors.emptyShade,
sectorLineWidth: 1.5,
},
},
};
}

export const EUI_CHARTS_THEME_LIGHT: EuiChartThemeType = createTheme(
lightColors
);
export const EUI_CHARTS_THEME_DARK: EuiChartThemeType = createTheme(darkColors);
// Build a static output of the EUI Amsterdam theme colors
// TODO: At some point, should Elastic Charts be able to inherit or create a theme dynamically from our theme provider?
const KEY = '_EUI_CHART_THEME_AMSTERDAM';
const builtTheme = buildTheme({}, KEY) as typeof EuiThemeAmsterdam;
const lightColors = getComputed(EuiThemeAmsterdam, builtTheme, 'LIGHT').colors;
const darkColors = getComputed(EuiThemeAmsterdam, builtTheme, 'DARK').colors;

export const EUI_CHARTS_THEME_LIGHT: EuiChartThemeType = createTheme({
...lightColors,
chartLines: shade(lightColors.lightestShade, 0.03),
chartBand: lightColors.lightestShade,
});

export const EUI_CHARTS_THEME_DARK: EuiChartThemeType = createTheme({
...darkColors,
chartLines: darkColors.lightShade,
chartBand: tint(darkColors.lightestShade, 0.025),
});

export const EUI_SPARKLINE_THEME_PARTIAL: PartialTheme = {
lineSeriesStyle: {
Expand Down
Loading

0 comments on commit fc30ca8

Please sign in to comment.