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

Fixing Amsterdam base line-heights #4229

Merged
merged 6 commits into from
Nov 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Added `EuiColorPaletteDisplay` component ([#3865](https://github.com/elastic/eui/pull/3865))
- Added `EuiColorPaletteDisplay` component ([#3865](https://github.com/elastic/eui/pull/3865))
- Added `initialFocusedItemIndex` support to `EuiContextMenuPanelDescriptor` ([#4223](https://github.com/elastic/eui/pull/4223))

**Bug fixes**
Expand All @@ -10,6 +10,10 @@
- Fixed a condition in `EuiInMemoryTable` to avoid mistaken assignment of `sortName` ([#4138](https://github.com/elastic/eui/pull/4138))
- Fixed bug in small `EuiImage`'s not respecting the optional sizes when `allowFullScreen` is set to true ([#4207](https://github.com/elastic/eui/pull/4207))

**Theme: Amsterdam**

- Fixed base `line-heights` for within `euiFontSize[size]()` SASS mixins ([#4229](https://github.com/elastic/eui/pull/4229))

## [`30.2.0`](https://github.com/elastic/eui/tree/v30.2.0)

- Added `labelWidth` and `descriptionDisplay` props to `EuiSuggestItem` ([#4180](https://github.com/elastic/eui/pull/4180))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ exports[`EuiFieldSearch props append is rendered 1`] = `
>
<eui-validatable-control>
<input
class="euiFieldSearch euiFieldText--inGroup"
class="euiFieldSearch euiFieldSearch--inGroup"
type="search"
/>
</eui-validatable-control>
Expand Down Expand Up @@ -134,7 +134,7 @@ exports[`EuiFieldSearch props prepend is rendered 1`] = `
>
<eui-validatable-control>
<input
class="euiFieldSearch euiFieldText--inGroup"
class="euiFieldSearch euiFieldSearch--inGroup"
type="search"
/>
</eui-validatable-control>
Expand Down
2 changes: 1 addition & 1 deletion src/components/form/field_search/field_search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ export class EuiFieldSearch extends Component<
{
'euiFieldSearch--fullWidth': fullWidth,
'euiFieldSearch--compressed': compressed,
'euiFieldSearch--inGroup': prepend || append,
'euiFieldSearch-isLoading': isLoading,
'euiFieldText--inGroup': prepend || append,
'euiFieldSearch-isClearable': isClearable && value,
},
className
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
.euiText {
background-color: $euiFormInputGroupLabelBackground;
padding: $euiFormControlPadding;
line-height: $euiFontSize !important;
line-height: $euiSize !important;
cursor: default !important; // pointer cursor on some form labels but not others is confusing

// If the next sibling is not the input, pull it closer to the text to reduce space
Expand Down
2 changes: 1 addition & 1 deletion src/global_styling/reset/_reset.scss
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ html {
}

body {
line-height: 1;
line-height: $euiBodyLineHeight;
}

*:focus {
Expand Down
3 changes: 2 additions & 1 deletion src/global_styling/variables/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ $euiFontSizeXL: $euiFontSize * nth($euiTextScale, 2) !default; // 28px
$euiFontSizeXXL: $euiFontSize * nth($euiTextScale, 1) !default; // 36px

// Line height
$euiLineHeight: 1.5;
$euiLineHeight: 1.5 !default;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one may still cause some issue within components, but we'll have to evaluate on a component by component basis.

$euiBodyLineHeight: 1 !default;

// Font weights
$euiFontWeightLight: 300 !default;
Expand Down
22 changes: 19 additions & 3 deletions src/themes/eui-amsterdam/global_styling/mixins/_index.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
// Import base theme first, then override
@import '../../../../global_styling/mixins/index';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the mixin updates were trickling down to the components when using importing the index file. So this new structure ensures that it does.

@import '../../../../global_styling/mixins/responsive';
@import '../../../../global_styling/mixins/shadow';
@import 'shadow';
@import '../../../../global_styling/mixins/size';
@import '../../../../global_styling/mixins/typography';
@import 'typography';
@import '../../../../global_styling/mixins/helpers';
@import '../../../../global_styling/mixins/states';
@import '../../../../global_styling/mixins/icons';

@import '../../../../global_styling/mixins/beta_badge';
@import '../../../../global_styling/mixins/button';
@import 'button';
@import '../../../../global_styling/mixins/form';
@import 'form';
@import '../../../../global_styling/mixins/header';
@import '../../../../global_styling/mixins/loading';
@import '../../../../global_styling/mixins/panel';
@import 'panel';
@import 'shadow';
@import 'form';
@import '../../../../global_styling/mixins/popover';
@import '../../../../global_styling/mixins/range';
@import '../../../../global_styling/mixins/tool_tip';
37 changes: 37 additions & 0 deletions src/themes/eui-amsterdam/global_styling/mixins/_typography.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Font sizing extends, using rem mixin
// All line-heights are aligned to baseline grid

@mixin euiFontSizeXS {
@include fontSize($euiFontSizeXS);
@include lineHeightFromBaseline(2);
}

@mixin euiFontSizeS {
@include fontSize($euiFontSizeS);
@include lineHeightFromBaseline(3);
}

@mixin euiFontSize {
@include fontSize($euiFontSize);
@include lineHeightFromBaseline(3);
}

@mixin euiFontSizeM {
@include fontSize($euiFontSizeM);
@include lineHeightFromBaseline(3);
}

@mixin euiFontSizeL {
@include fontSize($euiFontSizeL);
@include lineHeightFromBaseline(4);
}

@mixin euiFontSizeXL {
@include fontSize($euiFontSizeXL);
@include lineHeightFromBaseline(4);
}

@mixin euiFontSizeXXL {
@include fontSize($euiFontSizeXXL);
@include lineHeightFromBaseline(5);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ $euiFontSizeL: ceil($euiFontSize * 1.57); // 22px // h3
$euiFontSizeXL: floor($euiFontSize * 1.93); // 27px // h2
$euiFontSizeXXL: floor($euiFontSize * 2.43); // 34px // h1

$euiBodyLineHeight: 1.142857143; // 16px from a 14px base font size to ensure it aligns to our 16px grid

// Use 8px increments for base gridline
@function lineHeightFromBaseline($multiplier: 3) {
@return convertToRem(($euiSize/2)*$multiplier);
}
@mixin lineHeightFromBaseline($multiplier: 3) {
line-height: lineHeightFromBaseline($multiplier);
}

$euiTitles: (
'xxxs': (
Expand Down