Skip to content

Conversation

@kushthedude
Copy link
Member

Refers #3650

@auto-label auto-label bot added the fix label Jan 26, 2020
@iamareebjamal
Copy link
Member

@kushthedude kushthedude changed the title fix: Improvising the infinityLoader implemented fix: Improving the infinityLoader implemented Jan 26, 2020
@iamareebjamal
Copy link
Member

Still the same problem. Pushes the list up instead of down. Tested in Firefox and Chrome. Happens if you are absolutely at the end of the list. Also, have to scroll up and down multiple times before it starts to load stuff. Merging for now, but it is very annoying experience

@iamareebjamal
Copy link
Member

Also, load at least 12 or 15 speakers

@iamareebjamal
Copy link
Member

Well the sticking problem is still there and it only loads items when I am at absolute bottom, so threshold is not working as well. Will see tomorrow what can be done

@kushthedude kushthedude requested review from iamareebjamal and removed request for iamareebjamal January 29, 2020 09:48
@kushthedude
Copy link
Member Author

Well the sticking problem is still there and it only loads items when I am at absolute bottom, so threshold is not working as well. Will see tomorrow what can be done

It is a wait of <1 sec TBH. Also, I think micro-location should not be fetched, As we are already displaying micro-location in the session card. What do you suggest?
@iamareebjamal

@iamareebjamal
Copy link
Member

Let's take microlocation change in next release. About this PR, it doesn't change the experience for me unfortunately

@codecov
Copy link

codecov bot commented Feb 1, 2020

Codecov Report

Merging #3868 into development will decrease coverage by 0.16%.
The diff coverage is 77.77%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3868      +/-   ##
===============================================
- Coverage        21.98%   21.81%   -0.17%     
===============================================
  Files              460      460              
  Lines             4727     4726       -1     
===============================================
- Hits              1039     1031       -8     
- Misses            3688     3695       +7
Impacted Files Coverage Δ
app/controllers/register.js 27.58% <ø> (ø) ⬆️
app/services/loader.js 40% <ø> (ø) ⬆️
app/services/l10n.js 53.84% <ø> (ø) ⬆️
app/controllers/oauth/callback.js 0% <ø> (ø) ⬆️
app/components/account/application-section.js 0% <ø> (ø) ⬆️
app/controllers/public/index.js 0% <0%> (ø) ⬆️
app/components/forms/login-form.js 62.06% <100%> (ø) ⬆️
app/router.js 100% <100%> (ø) ⬆️
app/serializers/event.js 0% <0%> (-100%) ⬇️
app/services/auth-manager.js 51.78% <0%> (-5.36%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4708656...d8817eb. Read the comment docs.

@kushthedude
Copy link
Member Author

kushthedude commented Feb 1, 2020

Let's take microlocation change in next release. About this PR, it doesn't change the experience for me unfortunately

I tested it just now, It is at least better than the previous option without offset which was loading 6 speakers at a time 😕 . @iamareebjamal

</div>
{{/each}}
{{#infinity-loader infinityModel=model}}
{{#infinity-loader infinityModel=model triggerOffset=400 eventDebounce=50}}
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the changes here then

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!
Have we removed the codacy PR check?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

</div>
{{/each}}
{{#infinity-loader infinityModel=model}}
{{#infinity-loader infinityModel=model triggerOffset=400 eventDebounce=50}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{#infinity-loader infinityModel=model triggerOffset=400 eventDebounce=50}}
{{#infinity-loader infinityModel=model}}

@iamareebjamal
Copy link
Member

@kushthedude Release is being made. Do you want this included?

@kushthedude
Copy link
Member Author

I suggest including it but I am afk right now, can you please push the required changes, if any?

@iamareebjamal iamareebjamal merged commit 9ab1b7e into fossasia:development Feb 2, 2020
@kushthedude kushthedude deleted the infi branch February 2, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants