Add conditional shadow to sticky experiments column

[julieg18]

• Adds a shadow to the experiments column if you start scrolling horizontally.
• Takes off the gray border on the experiments column

Screen.Recording.2022-07-20.at.10.16.01.AM.mov

Fixes #2017

[shcheklein]

@julieg18 could you please add a video (on mac you can do video screenshots) and publish it to the description? plus put more context in the description if needed. Thanks! Good stuff.

[mattseddon]

[Q] Is the vertical line (from the video) in the design? Can it be removed?

[shcheklein]

Feedback - on the video shadow doesn't look like a shadow on my end?

And yes, we can now remove the gray vertrical line?

[julieg18]

Feedback - on the video shadow doesn't look like a shadow on my end?

Is it not showing up on a different theme, or are you not seeing it on any theme? Oh wait, I misunderstood. You mean on the actual video, the shadow isn't looking like a shadow. I believe the shadow looks more like a shadow on other themes but I chose to showcase the "dive bar" theme since you could see the shadow the most clearly.

[Q] Is the vertical line (from the video) in the design? Can it be removed?

Not sure what line you are referring to 🤔 The gray one?

[julieg18]

On further inspection, the shadow actually doesn't have any blur to it which is why it doesn't look very shadow-like. Going to look into adding the blur!

[julieg18]

Added a blur to the box shadow and removed the gray border on the experiments column!

[maxagin]

Yeah. Great stuff @julieg18 ! The video in the description has been updated I guess?

Should we add some transition to the box-shadow or is it fine as is?

transition very cool!

Should I adjust the header box-shadow so they are using the same kind like in Figma

No, please. The current shadows in the code are consistent. It's the same as notification shadow (I hope :) However, we may change it in the future.

[maxagin]

The transition for the header shadow is good I think. Let's use it for both then. Thanks @julieg18

[mattseddon]

[C] I think we need a better name for this "FirstCell". We would want to convey with the name that it contains controls/metadata about the rest of the row. I am not sure what the correct name would be.
Perhaps do a bit of research and follow up with a simple renaming PR. Rename everything around the "First" concept e.g <FirstCell/> etc.

[mattseddon]

[Q] Why do we need both of these criteria?
[Q] Can we pull out the index === 0 into a function so we don't duplicate it in two different places?

[julieg18]

[Q] Why do we need both of these criteria?

We need to select just one experiment column cell (aka one of the rendered TableHeaders) so we can add the observer to it. The PR does this by finding the first MergedHeaderGroups, then the first TableHeader. Personally, I felt like this solution felt overcomplicated but I couldn't think of an alternative 🤔

[Q] Can we pull out the index === 0 into a function so we don't duplicate it in two different places?

Sure!

[mattseddon]

Storybook has got a wacky diff. I checked the branch out locally and it looked fine. Please take a look before merging.

[julieg18]

Storybook has got a wacky diff. I checked the branch out locally and it looked fine. Please take a look before merging.

Looks like it is due to the storybook comparing the diff to an earlier version of the pr instead of main.

[codeclimate]

Code Climate has analyzed commit 1f17ec6 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

The test coverage on the diff in this pull request is 88.2% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.7% (0.0% change).

View more on Code Climate.

[shcheklein]

@julieg18 q - is the video up to date? I see that the horizontal shadow doesn't look the same now ... is it intentional?

[julieg18]

@julieg18 q - is the video up to date? I see that the horizontal shadow doesn't look the same now ... is it intentional?

The video is up to date and I didn't edit the horizontal shadow as far as I know... Can you specify what is exactly different on the horizontal shadow? They are looking the same from my end 🤔

[shcheklein]

From the video:

Top / horizontal shadow is not fading out? and looks a bit like a delimiter?

[julieg18]

If I'm understanding correctly, looking at the horizontal shadow from before this pr and how it looks currently, they seem to look the same. Though it does seem that neither is fading out very well 🤔

Current:

Before #2062 merge:

[mattseddon]

I think the part of the shadow in question might be the top of the vertical shadow:

[shcheklein]

@julieg18 sorry for the confusion. I mixed up the part of the tables's row with the shadow (also it starts with some delay after you scroll?) :) I see that it's the same more or less now.

[julieg18]

also it starts with some delay after you scroll?

Could be due to the transition animation or the code waiting till you've scrolled 15px 🤔 We could maybe move it to 5 or 10px instead?