Skip to content

Adds persistence and reloading of datatable pagination params#2112

Draft
pavgra wants to merge 12 commits into
masterfrom
issue-760
Draft

Adds persistence and reloading of datatable pagination params#2112
pavgra wants to merge 12 commits into
masterfrom
issue-760

Conversation

@pavgra
Copy link
Copy Markdown
Contributor

@pavgra pavgra commented Jan 23, 2020

fixes #760

@pavgra pavgra requested review from anthonysena and vlbe January 23, 2020 15:25
@anthonysena anthonysena added this to the V2.8.0 - Backlog milestone Jan 28, 2020
@anthonysena anthonysena self-assigned this Jan 28, 2020
Copy link
Copy Markdown
Collaborator

@anthonysena anthonysena left a comment

Choose a reason for hiding this comment

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

@pavgra just testing this out and it seems like what you've built here works well for the data table component itself. That said, it looks like perhaps the other parts of the application need to be more aware of the state of the data table. Take for example the following workflow:

  • Navigate to the cohort definition list via the left-hand menu
  • Filter the list using the search which results in a URL like: http://localhost:81/atlas/#/cohortdefinitions?dtSearch_-1258431457=ehden&dtPage_-1258431457=0
  • Open a cohort definition from that list
  • Close the cohort definition

I'd expect that closing the cohort definition would preserve the query string parameters required to filter the data table contents but that seems to get lost in the process. Could you take a look or let me know if you need any further details? Thanks!

@pavgra
Copy link
Copy Markdown
Contributor Author

pavgra commented Feb 13, 2020

@anthonysena , I do not think that this case can be implemented in a straightforward and clean way:

  • after you navigated to a specific cohort definition, you may go over tabs back and forward, go into vocabulary search to attach concepts, etc - so, it is impossible to assign a browser's back button behavior to the close button
  • therefore, we would need to store pagination parameters of the previous page:
    • either we can attach them to cohort definition URL - that would be /cohortdefinition/111?searchParamOfTableFromPrevPage=... - but that would make links really ugly and the logic very non-trivial
    • or we could put the params into sessionStorage; however, that would lead to the different behavior of the same page for different users, i.e. you wouldn't be able to share a link and expect the app to behave in the same way (since it would depend on your local data)

@anthonysena
Copy link
Copy Markdown
Collaborator

I'd like to confirm with @gklebanov around the functional requirements for #760 before we merge this into master.

@anthonysena anthonysena marked this pull request as draft May 12, 2020 13:07
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.

when cohort, concept set is opened and then closed, ATLAS resets the last page that was active

3 participants