Skip to content

A few adjustments#3

Merged
johnbarrierwilson merged 3 commits intojohnbarrierwilson:amsterdam/bordersfrom
cchaos:johnbarrierwilson/amsterdam/borders
May 14, 2020
Merged

A few adjustments#3
johnbarrierwilson merged 3 commits intojohnbarrierwilson:amsterdam/bordersfrom
cchaos:johnbarrierwilson/amsterdam/borders

Conversation

@cchaos
Copy link
Copy Markdown

@cchaos cchaos commented May 14, 2020

No description provided.

opacity: .2;
}

$arrowShadowCompensation: $euiSizeXS;
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

From what I could tell the 5px values you had before were more than enough to reveal the shadow. We really want to use our sizing variables as much as possible, so I tested with our XS value (4px) and it also seems enough to reveal the shadow.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍 That's a good idea.

@johnbarrierwilson johnbarrierwilson merged commit aa5dd37 into johnbarrierwilson:amsterdam/borders May 14, 2020
johnbarrierwilson pushed a commit that referenced this pull request Mar 3, 2022
…to` (elastic#5281)

* Remove isHeightSame check

* Remove same height check from shouldComponentUpdate

- if we're not checking for sameHeight in componentDidUpdate, we also likely don't need this anymore, and all dynamic user changes appear to work without this code

* [Proposed refactors]

- Rename `recalculateRowHeight` to `setAutoRowHeight`

- DRY out setAutoRowHeight to be reusable by both the resize observer and during props update (NB: This leads to some repetition with the height being obtained twice, but will not be an issue shortly as I'll be refactoring/removing the cell wrapper observer in a future PR)

- Refactor rowHeightUtils.isAutoHeight to accept an undefined rowHeightsOptions

* Improve unit tests

* Add changelog entry

* [PR feedback] Fix `isAutoHeight` false positive
- occurs if the `defaultHeight` is auto but a specific `rowHeights` line override is not auto

TODO: Write a unit test for this in the future

* [PR feedback/Discover testing] Trigger component update when rowHeightsOptions changes

* [cleanup] Remove now-unused compareHeights util

+ clean up mock rowHeightUtils as well

* [revert] rename setAutoRowHeight back to recalculateRowHeight

- lineCount in elastic#5284 will shortly be using the row height cache in addition to auto, so I'm making the fn name less specific again

* Add enqueue/timeout to recalculateRowHeight updates

- to avoid race conditions with cache being cleared

* Remove clearHeightsCache completely

* Unset row+column height values on unmount (#3)

* Add unit test for new unsetRowHeight

+ add optional isAutoHeight check

* Remove hidden columns from row height cache

* Early return in getRowHeight

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants