►
Description
In this session we try to debug a failing feature spec of an MR that attempts to replace jQuery scrolling with DOM based scrolling.
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43167
Test: `spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb`
A
A
A
B
So
I
had
some
time
last
week
and
started
picking
it
up
and
the
main
thing
that
I
did
was
fix
all
the
jquery
think
two
animation
stuff,
so
that
is
all
gone
by
now
and
then
I
look
what
is
still
missing
to
move
to
check
very
slim,
because
this
is
definitely,
I
think,
it's
pretty
percent
smaller
than
actual
jquery
yeah.
So
I
saw
this,
mr
from
my
team
that
he
started
to
move
away.
B
They
just
did
basically
the
move
and
I
now
started
fixing
specs
and
I
also
rewrote
because
what
we
are
doing
right
now.
We
are
imitating
browser
behavior
with
the
scroll
to
element
and
I'm
now
using
scroll
into
view
and
by
from
the
actual
classic
javascript
api.
I
want
the
element
only
if
we
are
having
an
offset
set
to
the
function,
then
we
are
using
still
the
measurements
that
we
have
in
place
and
almost
everything
speaks
apart
from
one
aspect:
that's
failing
at
the
moment.
A
B
And
I
tried
so
many
times
to
fix
this.
It
seems
my
gut
feeling
says
a
little
bit
in
the
direction.
Oh,
no
and
now
more
is
failing.
There
are
more
chests
since
I
moved
just
today
away
from
the
old
implementation,
because
I
said
now,
perhaps
it's
simply
due
to
the
fact
that
we
are
trying
to
calculate
all
the
time
which
definitely
with
old
jquery.
We
are
trying
to
calculate
so.
There
are
also
some
a
couple
of
chest
stacks,
but
they
are
failing.
Clearly,
the
main
stuff
is
the
rsp
okay.
A
C
A
D
A
D
A
Yeah
I
can,
I
can
bump
it
up.
Oh
man,
oh
man,
okay,
we'll
try
this
out.
This
might
be
too
big,
but
all
right,
I've
checked
it
out.
I'm
gonna
make
sure
I'm
on
the
the
latest
one.
But
then
I'm
gonna
do
my
my
thing.
So
I
can
see
the
changes
of
the
mr
and
status
okay
and
you
send
a
link
to
the
failing
r
specs,
while
that's
loading,
here's
the
failing
r
spec.
Oh,
my
goodness,.
D
B
B
A
A
D
A
A
B
A
A
D
Actually
obscured
checks
that
it's
in
the
center
of
the
viewport
and
not
visible
at
the
same
time.
So
this
is
this
is
a
complete
thing,
obscure
the
checks
one
again,
if
the
element
the
element
is
currently
in
the
viewport,
so
if
it's
clickable
but
not
visible
according
to
the
there
will
be
dock.
Well,
that's
that's
weird!
D
B
A
D
A
Maybe
it's
doing
stuff,
I
think
it
may
be
start.
Do
you
think?
Is
it
server
or
do
you
think
it's
start?
It
might
be
start,
but
anyways,
I'm
just
gonna
run
because
it's
what
I
usually
do,
but
while
that's
also
going,
we
can
also
check
it
out
in
prod
once
this
thing
is
or
not
pro
but
and
the
actual
application.
Once
this
thing
finishes.
Oh
here
we
go
great.
A
A
B
E
B
Jquery
scroll
to
and
then
it
was
using
the
windows
scroll
to,
and
I
basically
now
extended
it
to
also
you
scroll
into
view
so
that
it's
it
should
be
safer
because
then
the
browser
takes
over
all
that.
If
you
take
a
look
at
all
that
is
happening
on
calculating
which
bar
is
using,
how
much
space
is
so
that's.
The
main
change
is
just
now
replacing
jquery
scroll
to
in
all
instances.
A
A
B
A
A
A
A
B
B
A
D
B
D
A
A
D
A
D
A
A
A
H
F
A
F
A
D
A
A
Yeah,
I
would
because
we
don't
even
know
we
could
do
like
a
manual
wait
like.
Let's
make
sure
we
get
all
the
files
that
we
expect
you
know
and
then,
but
but
clearly
even
we're.
Not
our
test
is
failing
it
seemingly
for
a
good
reason.
I
think,
because
if
we,
if
we
run
it,
it's
not
showing
the
note-
and
I
think
somehow
we
want
to
make
sure
it
shows
the
note.
D
A
A
A
A
A
B
But
the
problem
with
the
offset
is
that
we
are
calculating
all
the
time
different
sizes
of
nav
bars
and
if
we
have
scroll
into
view
we
hand
this
over
to
the
browser.
It
should
work
yeah.
This
is
set
up
for
failure
in
the
sense
of
maybe
simply
the
specific
size
that
we
had
in
the
spec
has
worked
so
far
because
we
are
like
we
don't
have
yet
the
size
or
any
offset
of
the
element,
but
we
are
simply
calculating
a
couple
of
offsets
plus
zero,
and
then
we
still
land
quite
close
to
it.
A
E
A
But
all
the
batch
of
diffs
are
loaded
and
we're
not
going
to
be
bumping
anymore.
So
that's
one
approach
is
we
want
to
make
sure
they're
all
loaded.
The
other
thing
would
be.
Let's
just
make
sure
that
at
the
note
there's
not
going
to
be
any
loaded
above
it.
If
somehow
we
had
that
information,
but
I
am
wanting
to
check
this
out
on
master
or
just
without
these
changes,
since
it
is
a
couple
of
files,
I'm
just
gonna
undo
these
this
bit
and
I
can
just
revert
it
back
later.
A
Lazy,
we
do
it:
okay,
good
good,
just
making
sure
it's
yeah,
rather
than
one
humongous
request.
We
try
to
show
the
diffs
being
built
up,
so
we
just
send
it
and
a
sequence
of
stuff,
but
it's
not
like
when
we
should
show
it,
then
we
start
skeleton
loading
or
whatever
we're
not
like
responsive
to.
In
that
sense,
that's
what
you
mean.
H
F
F
F
It's
it's
can't
hear
me
anymore,
you
know,
and
it
won't
do
anything.
Okay,
damn
it
I'm
sorry.
I
was
trying
to
save
you
here.
No,
no,
it's
fine!
No!
The
the
thing
is
that,
via
my
the
local
state,
I'm
living
in
goes
into
corona
lockdown
on
monday,
and
so
I
would
be
alone
in
the
office
right
now
anyway,
and
so
I
said
you
know
with
the
new
baby
and
everything
it's
better.
If
I'm
just
here
until
the
lockdown
until
mid-january,
I
will
just
stay
at
home.
Just.
D
A
F
A
D
F
So
if
you
have
like
a
bunch
of
commits-
and
you
have
the
commit
hash,
it
scrolls
down
to
the
commit
in
the
commit
list
or
something
like
that-
and
I
don't
know
if
it
even
is
the
node
scrolling.
So
we
should
really
check
which
function.
Does
the
node
scrolling
it's
time
to
put
some
console
log
around.
A
A
E
H
A
A
H
A
F
A
D
H
A
D
So
I
may
have
a
theory
why
this
work
with
jquery-
I
was
looking
into
the
source
of
scroll
to
inside
it.
Uses
animate
and
animate
is
actually
a
loop
right
and
it
does
a
callback
that
we
call
itself
again.
So
it
may
cause
itself
again
until
it
actually
finds
the
element
right.
So
it's
chasing
it
across
the
page.
F
F
A
But
it's
like
it's
possible
that
we
have
other
situations
like
this
and
it
would
be
cool
to
have
a
context
agnostic
scroll
to
this
thing,
and
I
don't
have
it
doesn't
matter
if
there's
other
things
going
to
be
happening
above
or
below
it
or
whatever
it
just
knows.
We
got
to
get
this
thing
into
the
view.
I
could
see
that
example
yeah,
let's
set,
can
you
make
that
pain
bigger?
So
we
can
see
the
rest.
D
A
A
D
F
A
D
A
D
A
Yeah,
okay,
here's
all
the
changes.
Let
me
stash
pop.
Oh,
I
got
context
oh
gosh
because
I
did
add
stuff.
Let
me
not
stash
pop,
then
I
was
just
adding
console
logs.
So
this
this
yeah
yeah.
That's
no
problem
just
discard
it
yeah,
I'm
just
gonna
discard
it,
but
we
do
need
this
live
debug.
So
I'm
gonna
do
that
one.
E
A
A
I
A
D
A
D
D
A
A
And
it's
all
it's
all
anchor
tags
and
stuff,
so
I
think
it's
you
know
we
don't
even
need
to
talk
through
components.
It's
just
hey
just
need
to
scroll
to
this
element,
but
I
can
only
do
it
this
time.
I
really
feel
yeah.
This
doesn't
need
to
be
handled
at
the
note
level,
but
I
imagine
we
did
it
at
the
note
level,
because
notable
note
is
used
for
issues,
merge,
requests
everything
uses
this.
So
it's
any
time
we're
trying
to
load
a
note.
A
A
F
I
mean
I
mean
to
me,
it
would
be
interesting
if
we
could
make
use
of
any
of
any
dom
element
or
something
right
that
you,
for
example,
say
hey.
I
want
to
have
the
scrolling
behavior
and
I'm
going
to
keep
track
until
dom
content
loaded
or
until
I
don't
know
what
right
in
the
future
and
then
I'm
going
to
stop
the
tracking
thing
right
or
maybe
I
stop
the
thing
once
the
user
has
like
scrolled
by
themselves
or
something.
A
A
D
Well,
we
can,
we
can
have
a
small
callback
thingy
that
looks
for
the
element,
it
checks
the
position
and
I
don't
know
after
three
times
that
it
run
100
milliseconds
one
after
the
other.
The
element
is
still
there,
it
called
it
done.
I
mean
it's,
it's
all
open
source.
Can
we
just
I
mean
we
don't
I
can't
I
can
send
you
the
article.
I
think
this
is
this.
Is
the
one
jquery
scroll
to
I
found
it
on
github?
F
D
A
D
G
D
F
F
If
you
had
a
duration
before
you
probably
want
to
have
smooth
scrolling,
so
we
do
the
smooth
scrolling
and
rather
than
a
real
timeout.
Do
you
see
that
like
yeah,
because
auto
is
like
the
quick
like
scroll
immediately
right,
so
the
only
thing
that
we
do
with
duration
is
yeah.
Okay,
let's
do
a
smooth
scroll
if
you
wanted
to
because
it
looks
like
an
animation,
but
it
doesn't
really
do
the
real
time
out
thingy,
but
we
were
quite
coupled
to
that
delay
here.
That's
that's
the
takeaway.
F
H
D
D
F
I
mean
we
now
have
a
source
code
team.
Couldn't
we
just
create
a
bug
and
be
like
hey?
The
node
scrolling
is
not
working
as
well.
You
should
look
into
the
implementation
of
that
for
the
mrd
view,
and
please
do
that
soon
and-
and
you
know,
because
the
behavior
is
bad
right
now
and
it
will
just
be
as
bad
if
we
now
put
a
time
out
of
200
milliseconds
or
something
right.
A
F
Move
the
gain
we
get
is
that
we
are
able
to
throw
like
a
third
of
jquery
away
or
something
like
that
and
that
by
doing
it
we
are
putting
some
more
eyes
on
it
and
can
create
like
a
better
follow-up
issue
and
say:
hey
this
behavior
is
broken
right
away
right
now,
anyway,
please
go
and
fix
it.
I
don't
know.
A
Right,
but
with
no-
and
that
makes
sense,
but
we
are
able
to
justify
this
with
immediate
win
like
we're.
After
this,
mr,
we
can
throw
away
part
of
jquery,
yes,
okay,
I
think
that
makes
sense,
because
yeah
our
net,
our
net,
we're
still
netting
out
a
positive
because
we're
already
this
isn't
great,
and
I
want
to
confirm
this
by
actually
changing
this
to
zero.
So
I
want
to.
I
want
to
do
one
more
confirmation
to
that
we're
coupled
to
this
magic
number
by
moving
this
to
zero.
H
Yeah,
I
think
I
think
that's
what
I
wanted
to
do
as
well.
I
want
to
make,
because
we've
been
you've
been
at
this
for
a
while
and
it
jumped
in
a
little
bit
later.
I
want
to
make
completely
sure
that
we're
like
this
is
this
is
the
problem
exactly
I.
I
have
some
level
of
confidence
at
this
point,
but
but
it's
it's
always
good
to
see
it
yeah
spelled
out.
I
think
I
think
it's
a
good
call
ip
that
to
round
it
off
here
create
an
issue.
H
I
think
definitely
encourage
the
people
who
are
working
on
this
to
to
look
through
this
pairing
session
to
to
regain
the
same
conscious
text
that
has
been
built
up
here.
That
said,
if
we
do,
I
don't
think
we
have
minutes
left,
but
it
could
be
interesting
to
to
like
just
throw
off
as
a
final
gesture
like
what
would
be
what
would
be
a
good
way
to
to
solve
this.
A
Yeah-
and
I
think
I
I
think
what
our
guts
were
telling
us
at
the
beginning,
we
one
approach
is
letting
the
thing
that
has
the
information
kick
this
off.
The
note
is
just
looking
at
itself
and
doesn't
have
information
of
everything
else
that's
going
on,
so
we
need
to
move
that
up.
But
the
scary
thing
is
that
these
notes
aren't
just
used
for
merge,
requests,
it's
used
for
issues
and
everything
else,
and
so
moving
things
up
to
the
entry
point.
A
H
I
think
this
calls
for
a
like
a
general
solution,
a
a
component
that
handles.
I
do
like
that.
The
component
can
tell
I'm
ready,
but
I
want
to
leave
the
control
to
the
to
the
parent
yeah
and
to
make
the
decision
to
say
I'm
gonna
scroll
to
you
so
like
some
some
kind
of
negotiation
where,
where
you
say
I'm
ready
to
be
honest,.
F
F
So
the
question
is
like:
maybe
maybe
it
is
possible
to
create
such
a
function,
that
you
know
maybe
for
a
few
milliseconds
checks
if
it's
still
in
the
view
and
if
the
user
doesn't
have
hasn't
interacted
with
the
page
scrolls
to
it
again
and
otherwise
you
know
just
leaves
it
e
in
perfect.
I
don't
know.
A
Another
really
viable
option
as
well,
so
it's
gonna,
it's
gonna.
I
think
it's
worth
looking
into
into
it
for
sure
I
I
personally
like
having
smaller,
focused
resilient
systems.
I
think
is
better
than
when
we
just
make
our
domain
logic
really
large.
If
it's
like,
oh
well,
now
we
have
batch
diffs
and
we
have
all
these
other
stuff
going
on.
Maybe
we
could
reframe
the
problem
to
what
you're
describing
ip.
I
personally
like
that
because
of,
I
think,
the
resiliency
that
it
could
lead
to.
But
you
know
this
is
great
point.
A
A
A
But
that's
scary
and
it's
eve
and
it's
scary
too,
because
that's
going
to
be
a
flaky
that
that's
open
to
some
flakiness
and
spec
world
as
well
like
I'm,
not,
I
think
it's
a
good
call.
I
think
it's
a
good
call
for
this
next
step
because
that's
currently
what
we
have,
but
I
hope
we
don't
just
say
this
is
the
only
way
to
do
it.
A
A
F
A
A
A
F
A
I
think
part
of
it
too
might
be.
I
think
it
might
not
be
this
timing
stuff.
It
could
be
because
this
you
see
we're
we're
scrolling
at
the
body
level,
but
here
we're
doing
scroll
into
view,
and
so
this
is
going
to
take
into
account.
I
think
some
offset
stuff
that
are
we
do.
Are
we
even
using
that,
though,
do
you
guys
have
yeah,
I'm
not
using
it,
but
we
do
use
content
top
and
that's
a
big
deal,
yeah
content
top
we'll
we'll
do
it?
Here's
why?
F
To
be
honest,
like
I
think
we
have
a
pretty
good
point,
I
have
to
hop
off
as
well.
I
mean
I
started
dmr
like
eight
weeks
ago
so
yeah.
I
know
that
we
probably
need
more
debugging,
and
you
know
this.
Mr
viewer
has
needs
some
special
care
and
you
see
it's
probably.
The
smart
thing
to
do
is
to
have
the
old
end
to
the
new
implementation
and
migrate
bit
by
bit,
rather
than
everything
at
once,
even
though,
if
it
would
be
nice
to
migrate
everything
at
once,
yeah.
A
Yeah
yeah,
that's
and
that's
a
really
good
call
too
that'll
definitely
help
speed
things
along
so
well,
thanks
for
hopping
on
ip,
I
have
one
more
thing:
I'm
gonna
look
at
and
then
I'll
stop,
recording
and
michael.
If
you
gotta
hop
off,
go
for
it
or
if
you
wanna
hang
around,
I
can.
I
can
hang
around
yeah.
I
was
wanting
to
see
what
happens
if
we
just
call
this
behavior,
because
I
see
we're
not
even
targeting
the
elements
here
at
all.
A
A
H
Like
just
content
top,
but
that
seemed
and-
and
I
was
thinking
it's
a
big
deal
because
you
you
thought,
like
I
thought
you
meant
it
was
like
gonna-
do
a
reflow
or
something-
and
I
was
like
well
like
this
whole
thing.
It
was
about
reflows,
yeah
yeah,
that's,
but
I
did
not.
I
did
not
think
that
was
a
separate
function.
Function
that
does
all
sorts
of
things.
A
A
A
A
A
A
A
E
A
If
vue
was
like
doing
this
re-rendering
thing
and
had
to
create
a
whole
new
element
or
something
maybe
the
element
isn't
even
showing
up
in
the
dom
anymore.
When
we're
running
this,
because
one
difference
between
that
and
the
old
behavior
is
we're
not
scrolling
to
a
specific
element,
we
just
get
the
element
info
for.
Where
do
we
need
to
put
the
scroll
bar
right?
You
know
so,
when
I
did
this
so
forcing
this
condition
to
run
this
behavior,
it
actually
worked.
A
A
G
G
A
G
A
H
What
I'm
thinking
here
is
because
there
seems
to
be
some
amount
of
guessing
into
this
and,
like
constant
top,
does
something,
and
we
don't
know
what
and
just
bear
with
me
here.
I
I
know
right.
I
know
I
know,
but
I
like
it
might
actually
be
worth
the
effort
going
through
and
like
implementing
it,
how
we
want
it
to
yeah
like
starting
over,
like
like
and
and
and
again
see
separate
like
separate
issue,
but
like
remove
the
content
top
and
like
okay.
H
A
I
think
you're,
I
think,
you're
100
correct.
I
think
we
could
greatly
simplify,
what's
going
on
by
reshaping
the
problem
as
opposed
to
scroll
to
an
element
and
get
labs
ui
which
could
have
these
and
could
have
these
and
it's
a
very,
very
complex
problem.
If
we
reshape
it
to
scroll
to
an
element
which
may
or
may
not
be
hidden
on
the
screen
under
a
certain
period
of
time.
And
then
we
just
we
just
figured
out.
H
A
A
Yeah
totally
so
I'm
I'm
seeing
way
more
success
with
this
condition
than
with
this
one
and
I
kind
of
now.
If
we
had
lots
of
time,
I
would
want
to
debug
okay.
What
is
actually
l
does
even
exist
in
the
document
anymore,
because
what
we
saw
on
the
rspec
is
it
wasn't
going
anywhere.
So
wonder
why
that's
not
happening
the
other
thing
I
was
really
wanted
to
dive
into.
I
was
wanting
to
add
a
debugger
and
just
step
into
animate
and
just
see
what
is
actually
animate
doing
right.
E
A
A
A
A
A
A
Yeah,
it
doesn't
do
anything
okay,
I
wasn't
sure
if
it
gave
it
like
a
default
or
something
then
here's
the
animation,
which
is
a
function
to
do
the
stuff.
So
then
we
are
then
going
to
call
each
animation.
It
looks
like
so
we're
going
to
try
to
do
this
thing
or
we
can,
or
we
can
cue
it,
but
cueing.
We
don't
have
that's
not
in
our
options.
We
don't
cue
things.
I
don't
know.
Let's
add
a
watch
for.
F
A
A
H
A
A
A
E
H
A
A
A
A
A
Boom,
that's
in
that's
deep
in
the
animation
stuff,
so
I
want
to
check
this
out
when
I
step
into
here.
This
is
I
need
to
skip
this
one.
Let's
get
this
skip,
it's
trying
to
do
a
deep
copy
of
something-
and
I
don't
I'm
not
concerned
about
this
right
now.
Oh
gosh!
Okay,
let's
go
for
it
double
double
for.
A
A
Where
does
it
so
yeah?
What's
start
time?
There's
no!
Let's,
let's
see
it
is
deferring
it
yeah,
which
is
why
it
works.
There's
no
fx
now
so
we're
going
to
create
a
time
that
we
need
to
start
and
we
have
a
tick
function
which
is
yeah
yeah.
So
if
I
go
here,
I'm
going
to
I
put
a
break
point
at
our
tick
function.
A
A
H
I
I
do
like
my
my
expression
about
like
completely
rewriting
like
I
I
know
it's.
It's
like
it
might
not
necessarily
be
worth
the
effort,
but
I
think,
like
a
compromise
between
that
and
what,
if
he
is
suggesting
with,
like
you
know
the
mvp
that
gets
the
job
done
without
touching
all
like
all
the
different
things
I
I
think
is
a
good
is
a
good
step
forward,
because
that,
if
that
means
we
can
remove
a
lot
of
this
this
code,
then
at
least
we
can
start
there
and
then
like.
H
A
I
I
think,
that's
a
really
good.
I
I
like
it
because
the
problem
scope,
it's
a
it's
a
problem,
it's
a
hard
problem,
but
it's
the
problem,
scope
is
is
focused
and
it's
small
and
it's
to
the
point
where
and
because
it's
not
coupled
to
get
lab
context
means
we
can
just
use
it
whenever
we
need
it,
and
I
I
like
that
a
lot,
so
I
I
feel
like
that
would
be
a
good
approach.
A
I
can
understand
if
we're
wanting
a
win
right
now,
wanting
to
just
do
a
set
timeout,
which
is
which
is
technically
this
deferred
stuff
here
and
I'd
be
fine
with
that,
but
ideally
that
wouldn't
last
for
a
while
we'd
have
an
issue
for
we
need
to
solve
this
specific
problem
holistically
right.
But
this
is
interesting.
Definitely.