Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Changing the pagination in Direct users modal from buttons to infinit… #1855

Closed
wants to merge 8 commits into from
Closed

Changing the pagination in Direct users modal from buttons to infinit… #1855

wants to merge 8 commits into from

Conversation

JayaKrishnaNamburu
Copy link
Contributor

@JayaKrishnaNamburu JayaKrishnaNamburu commented Oct 9, 2018 •
edited by hanzei

Summary

Changing the behavior from pagination to infinite scroll for Direct Messages.

Ticket Link

MM-4687

Issue Link

5790

Checklist

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)
  • Has UI changes

@jasonblais jasonblais added the Work in Progress Not yet ready for review label Oct 10, 2018
@jasonblais
Copy link
Contributor

Thank you @JayaKrishnaNamburu! Looking forward to testing this out and when it's ready 👍

@JayaKrishnaNamburu JayaKrishnaNamburu changed the title WIP: Changing the pagination in Direct users modal from buttons to infinit… Changing the pagination in Direct users modal from buttons to infinit… Oct 11, 2018
@JayaKrishnaNamburu
Copy link
Contributor Author

Hi @jasonblais please review this :)

@jasonblais jasonblais added 1: PM Review Requires review by a product manager Setup Old Test Server Triggers the creation of a test server and removed Work in Progress Not yet ready for review Setup Old Test Server Triggers the creation of a test server labels Oct 12, 2018
@mattermost mattermost deleted a comment from mattermod Oct 12, 2018
@mattermost mattermost deleted a comment from mattermod Oct 12, 2018
@mattermost mattermost deleted a comment from mattermod Oct 12, 2018
@jasonblais
Copy link
Contributor

Thanks @JayaKrishnaNamburu! Can you help rebase and I'll re-generate a test server after your latest changes

Big thanks for your help!

@JayaKrishnaNamburu
Copy link
Contributor Author

JayaKrishnaNamburu commented Oct 13, 2018 •
edited

Thanks @JayaKrishnaNamburu! Can you help rebase and I'll re-generate a test server after your latest changes

Big thanks for your help!

Sure @jasonblais but yesterday I tried rebasing with master but it was saying everything was updated, let me check once again 👍

@JayaKrishnaNamburu
Copy link
Contributor Author

Hi @jasonblais done with rebasing 👍

@esethna esethna added the Setup Old Test Server Triggers the creation of a test server label Oct 14, 2018
@jasonblais
Copy link
Contributor

Hey @JayaKrishnaNamburu, thanks again for your contribution!

It's definitely a step in the right direction. Two notes below. Let us know if you have questions on either one.

1 - I think your previous rebase picked up changes not directly related to this PR. Here's a guide on how to rebase your code using Git if you'd be open to trying again? https://www.digitalocean.com/community/tutorials/how-to-rebase-and-update-a-pull-request

2 - I see the paging taking place as I scroll on the list

image

However, it appears to bring me back to the top after the loading is completed. Video of the observed is below.

https://drive.google.com/file/d/1YxRX6IxRQ4CemQeCdbTKd5na_lyYrF-e/view?usp=sharing

The expected is that the user stays in the same place as before, and sees the next page of users loaded.

@jasonblais
Copy link
Contributor

/cc @esethna on above as you might have UX notes to share too.

@JayaKrishnaNamburu
Copy link
Contributor Author

JayaKrishnaNamburu commented Oct 15, 2018 •
edited

Hi @jasonblais I think something is missing in the branch because during Dev after pagination it started scrolling to the next page instead of going to top, let me check the issue and I will update the branch 👍 Thank you 😃

@esethna esethna added the Awaiting Submitter Action Blocked on the author label Oct 15, 2018
@JayaKrishnaNamburu
Copy link
Contributor Author

Hi, @jasonblais can you please review the functionality once again, I made a few changes.
This is the current behavior --> https://drive.google.com/file/d/1L3HUBVt0BojJWks0oPK25HrpPcxiO1dJ/view?usp=sharing

@jasonblais jasonblais removed Awaiting Submitter Action Blocked on the author Setup Old Test Server Triggers the creation of a test server labels Oct 16, 2018
@mattermost mattermost deleted a comment from mattermod Oct 16, 2018
@mattermost mattermost deleted a comment from mattermod Oct 16, 2018
@mattermost mattermost deleted a comment from mattermod Oct 16, 2018
@jasonblais jasonblais added the Setup Old Test Server Triggers the creation of a test server label Oct 16, 2018
@jasonblais
Copy link
Contributor

Thank you @JayaKrishnaNamburu! Functionally the scrolling works and I didn't find major bugs (apart from a known issue with creating a new direct message channel https://mattermost.atlassian.net/browse/MM-12570).

Great work.

I'm asking our UX/UI team to help review the loading screen and see if there are potential improvements we can make. The existing one works well, but there might be an opportunity to make the experience even smoother, such as showing the loading the indicator at the bottom of the list similar to our mobile apps.

image

@hanzei hanzei removed Lifecycle/1:stale Setup Old Test Server Triggers the creation of a test server labels Feb 6, 2019
@mattermost mattermost deleted a comment from mattermod Feb 6, 2019
@mattermost mattermost deleted a comment from mattermod Feb 6, 2019
@esethna
Copy link
Contributor

esethna commented Feb 7, 2019

@JayaKrishnaNamburu just checking in on this PR, can we help clarify anything?

@JayaKrishnaNamburu
Copy link
Contributor Author

@JayaKrishnaNamburu just checking in on this PR, can we help clarify anything?

Hi @esethna just changed the threshold calculation from a constant value to percentages, it would be helpful to if you check the PR and let me know if i miss out something 😊

@mattermost mattermost deleted a comment from mattermod Feb 8, 2019
@esethna esethna added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Feb 8, 2019
@hanzei hanzei added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Feb 20, 2019
@mattermod
Copy link
Contributor

Setup Test Server label detected. Spinmint test server created if build succeeds (checks pass and no conflicts with base branch).

@mattermod
Copy link
Contributor

Spinmint test server created at: https://i-010ed399b775c09e7.test.spinmint.com

Test Admin Account: Username: sysadmin | Password: sysadmin

Test User Account: Username: user-1 | Password: user-1

Instance ID: i-010ed399b775c09e7

@hanzei hanzei requested a review from esethna February 20, 2019 09:48
@esethna
Copy link
Contributor

esethna commented Feb 21, 2019

@sudheerDev @JayaKrishnaNamburu sorry I'm not noticing any infinite scroll on the Direct Messages + modal anymore. Looks like the entire list of users is loaded immediately?

inifinite

@mattermod
Copy link
Contributor

Spinmint test running for more than 7 days. This test server was terminated.

@amyblais amyblais removed the Hackfest label Mar 13, 2019
@esethna esethna added Lifecycle/1:stale and removed 1: PM Review Requires review by a product manager Setup Old Test Server Triggers the creation of a test server labels Mar 22, 2019
@jasonblais
Copy link
Contributor

Hey @JayaKrishnaNamburu - did you have any questions on the above feedback? Let us know if we can help

@hanzei
Copy link
Contributor

hanzei commented May 7, 2019

Hey @JayaKrishnaNamburu,

Thank you for this contribution. This PR has been inactive for quite a while. I'm closing this one. Let me know, if you still want to work on this.

@hanzei hanzei closed this May 7, 2019
@lindalumitchell lindalumitchell added the Tests/Not Needed Does not require new release tests label May 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hanzei hanzei hanzei left review comments

@sudheerDev sudheerDev sudheerDev requested changes

@jasonblais jasonblais jasonblais approved these changes

@asaadmahmood asaadmahmood Awaiting requested review from asaadmahmood

@esethna esethna Awaiting requested review from esethna

Assignees

@esethna esethna

Labels
Llifecycle/3:orphaned Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
No milestone
8 participants
@JayaKrishnaNamburu @jasonblais @esethna @sudheerDev @hanzei @mattermod @lindalumitchell @amyblais