WIP: Save limit value in localStorage #3850

polRk · 2020-02-07T11:00:40Z

Description

Put a separate option in the settings tab.
Save pageSize default value in localStorage.
Load page Size default value in Browser Rows view

Affected components

Console

Related Issues

#3849

Solution and Design

hasura-bot · 2020-02-07T11:00:42Z

Beep boop! 🤖

Hey @polRk, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! 😎

hasura-bot · 2020-02-07T11:13:05Z

Review app for commit 7609d37 deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-7609d37f

netlify · 2020-02-07T11:19:09Z

Deploy preview for hasura-docs ready!

Built with commit ecf083b

https://deploy-preview-3850--hasura-docs.netlify.com

netlify · 2020-02-07T11:20:27Z

Deploy preview for hasura-docs ready!

Built with commit 10ee273

https://deploy-preview-3850--hasura-docs.netlify.com

polRk · 2020-02-08T10:02:22Z

@hasura-bot deploy

rikinsk · 2020-02-11T07:21:31Z

/heroku deploy

hasura-bot · 2020-02-11T07:22:27Z

Review app for commit 5e5a2fc deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-5e5a2fcc

rikinsk · 2020-02-11T07:29:41Z

@polRk Thanks for taking this issue up and contributing.

I just had some feedback on the UX. Instead of adding a new page in the setting section to set the default pageSize value, it would just suffice to persist the page size value when the user updates the page size on the browse rows page itself. If user sets page value to 20, then we persist that value and show 20 results always from then on.

polRk · 2020-02-11T11:14:31Z

@rikinsk Looks pretty good. I'll try to finish it soon

polRk · 2020-02-11T12:19:25Z

/heroku deploy

hasura-bot · 2020-02-11T12:19:26Z

@polRk you don't have enough permissions to execute this command

hasura-bot · 2020-02-11T12:26:55Z

Review app for commit 324520f deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-324520fc

polRk · 2020-02-11T12:39:25Z

After update, i see 2 requests ( with limit 10, and limit from localStorage )

hasura-bot · 2020-02-11T13:08:46Z

Review app for commit 1f914bf deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-1f914bfc

polRk · 2020-02-11T13:44:22Z

@beerose Can you make a review ?

beerose · 2020-02-11T13:47:06Z

I'll do the review first thing tomorrow morning.

…

On Tue, 11 Feb 2020 at 14:44, Vladislav Polyakov ***@***.***> wrote: @beerose <https://github.com/beerose> Can you make a review ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3850?email_source=notifications&email_token=ACE2ABOPEPOXNXYAFBKSGOTRCKTTPA5CNFSM4KRMSG62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELMOQSQ#issuecomment-584640586>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACE2ABP3J74BEXO32FTZDJTRCKTTPANCNFSM4KRMSG6Q> .

beerose

Hi @polRk. Thank you for your contribution!

Your code is working fine, but I see some problems with the implementation.

The content of the DataState.js is a pure state of the app, where we don't want to introduce any side effects. Thus, it's not the place for accessing the local storage.

Instead, take a look at this file and componentDidMount method: https://github.com/polRk/graphql-engine/blob/master/console/src/components/Services/Data/TableBrowseRows/FilterQuery.js#L173
Here, we are setting the default values, so this is the proper place for taking the limit from localStorage and passing it as a default limit:

dispatch(setDefaultQuery({ where, order_by, limit }));

Another thing is how you access the Local Storage. We don't want to do it directly from the components or reducers. Instead, we rather create some abstractions for Local Storage. Take a look at this file: https://github.com/hasura/graphql-engine/pull/3753/files#diff-7d6f61aee8febe1d54e2e517ca909792R1 .

It would be ideal if you'd make this PR a part of #3753 (by changing the base), which adds similar functionality -- it also makes some settings persistent.

beerose · 2020-02-12T08:58:24Z

console/src/components/Services/Data/DataState.js

  query: {
    columns: [],
-    limit: 10,
+    limit: parseInt(localStorage.getItem('console:queryLimit') || 10, 10),


Suggested change

limit: parseInt(localStorage.getItem('console:queryLimit') || 10, 10),

limit: 10

beerose · 2020-02-12T08:58:40Z

console/src/components/Services/Data/DataState.js

 const defaultCurFilter = {
  where: { $and: [{ '': { '': '' } }] },
-  limit: 10,
+  limit: parseInt(localStorage.getItem('console:queryLimit') || 10, 10),


Suggested change

limit: parseInt(localStorage.getItem('console:queryLimit') || 10, 10),

limit: 10

beerose · 2020-02-12T08:59:59Z

console/src/components/Services/Data/TableBrowseRows/ViewActions.js

        ...defaultViewState,
        query: {
          columns: currentColumns,
-          limit: 10,


Don't delete this line.

beerose · 2020-02-12T09:00:09Z

console/src/components/Services/EventTrigger/PendingEvents/ViewActions.js

              where: { delivered: false, error: false },
            },
          ],
-          limit: 10,


Don't delete this line.

polRk · 2020-02-12T10:47:28Z

@beerose How can i change the base ?

hasura-bot · 2020-02-12T11:24:46Z

Review app for commit 05e2ede deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-05e2eded

beerose · 2020-02-12T11:54:35Z

@polRk You would need to create a new PR and choose my fork and browse-table-improvements branch.

But what happened here?

polRk · 2020-02-12T12:11:25Z

But what happened here?

I made a rebase to get new code from the master branch. Standard practice

beerose · 2020-02-12T12:27:26Z

But what happened here?

I made a rebase to get new code from the master branch. Standard practice

I think something went wrong here because you have 706 commits to merge.
What you could do now is:

Fetch the newest Hasura master.

git remote add hasura git@github.com:hasura/graphql-engine.git

git fetch hasura master

Do a soft reset:

git reset --soft hasura/master

Commit your changes.

git commit -m 'Save limit value in localStorage'

polRk · 2020-02-12T14:48:37Z

I think something went wrong here because you have 706 commits to merge.

I rebased wrong master. Solved, thank you

hasura-bot · 2020-02-12T15:02:12Z

Review app for commit 1df76bb deployed to Heroku: https://hge-ci-pull-3850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3850-1df76bb8

CLAassistant · 2020-03-17T12:27:17Z

All committers have signed the CLA.

beerose · 2020-04-07T13:31:29Z

Hi @polRk! It's been a while since your last commit, so I want to ask if you still plan to work on this PR?

polRk · 2020-04-07T13:36:39Z

Hi @beerose! I don’t have enough time for this PR. I want to make hasura console better, if someone can to do this faster than me, it will be good. Sorry for waiting

beerose · 2020-04-07T19:15:53Z

Closing in favor of #4323.

hasura-bot · 2020-04-07T19:15:55Z

Review app https://hge-ci-pull-3850.herokuapp.com is deleted

hasura-bot · 2020-04-07T19:15:56Z

Beep boop! 🤖

Hey @polRk!

Sorry that your PR wasn’t merged.

Do take a look at any of the other open issues to see if you’d like to take something up! We’re around on Discord if you have any questions 😄

polRk requested a review from beerose as a code owner February 7, 2020 11:00

polRk changed the title ~~WIP: save pageSize default value in localStorage~~ Save limit value in localStorage Feb 11, 2020

beerose suggested changes Feb 12, 2020

View reviewed changes

polRk force-pushed the patch-1 branch from 1f914bf to 73054b1 Compare February 12, 2020 10:58

polRk requested review from lexi-lambda, marionschleifer and shahidhk as code owners February 12, 2020 10:58

Save limit value in localStorage

1df76bb

polRk force-pushed the patch-1 branch from 05e2ede to 1df76bb Compare February 12, 2020 14:47

marionschleifer added c/console Related to console community-contrib 🙏🏽 labels Feb 12, 2020

polRk changed the title ~~Save limit value in localStorage~~ WIP: Save limit value in localStorage Feb 14, 2020

rikinsk assigned beerose Apr 7, 2020

beerose closed this Apr 7, 2020

polRk deleted the patch-1 branch April 8, 2020 08:34

	limit: parseInt(localStorage.getItem('console:queryLimit') \|\| 10, 10),
	limit: 10

WIP: Save limit value in localStorage #3850

WIP: Save limit value in localStorage #3850

Uh oh!

Conversation

polRk commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Affected components

Related Issues

Solution and Design

Uh oh!

hasura-bot commented Feb 7, 2020

Uh oh!

hasura-bot commented Feb 7, 2020

Uh oh!

netlify bot commented Feb 7, 2020

Uh oh!

netlify bot commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

polRk commented Feb 8, 2020

Uh oh!

rikinsk commented Feb 11, 2020

Uh oh!

hasura-bot commented Feb 11, 2020

Uh oh!

rikinsk commented Feb 11, 2020

Uh oh!

polRk commented Feb 11, 2020

Uh oh!

polRk commented Feb 11, 2020

Uh oh!

hasura-bot commented Feb 11, 2020

Uh oh!

hasura-bot commented Feb 11, 2020

Uh oh!

polRk commented Feb 11, 2020

Uh oh!

hasura-bot commented Feb 11, 2020

Uh oh!

polRk commented Feb 11, 2020

Uh oh!

beerose commented Feb 11, 2020 via email

Uh oh!

beerose left a comment

Choose a reason for hiding this comment

Uh oh!

beerose Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

beerose Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

beerose Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

beerose Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

polRk commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hasura-bot commented Feb 12, 2020

Uh oh!

beerose commented Feb 12, 2020

Uh oh!

polRk commented Feb 12, 2020

Uh oh!

beerose commented Feb 12, 2020

Uh oh!

polRk commented Feb 12, 2020

Uh oh!

hasura-bot commented Feb 12, 2020

Uh oh!

CLAassistant commented Mar 17, 2020

Uh oh!

beerose commented Apr 7, 2020

Uh oh!

polRk commented Apr 7, 2020

Uh oh!

polRk commented Feb 7, 2020 •

edited

Loading

netlify bot commented Feb 7, 2020 •

edited

Loading

polRk commented Feb 12, 2020 •

edited

Loading