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

A few adjustments #3

Conversation

cchaos
Copy link

@cchaos cchaos commented May 14, 2020

No description provided.

}

$arrowShadowCompensation: $euiSizeXS;
Copy link
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.

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