Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M3-3780 Add "Show All" option to Domain Records #6008

Merged

Conversation

johnwcallahan
Copy link
Contributor

@johnwcallahan johnwcallahan commented Jan 14, 2020

Description

This uses the same local storage key as Linodes, Domains, Volumes, and NodeBalancers, so if you select "Show All" on one of those pages, you'll see all of your Domain Records listed as well.

If we don't like this approach, we could make the Domain Records pagination independent or not saved in local storage at all.

Side note: there are some serious performance problems when you have a ton of records on the page (at least running a dev server). E.g. try opening a drawer... all the record rows are re-rendered. Creating a ticket to address separately.

Type of Change

  • Non breaking change ('update', 'change')

Note to Reviewers

Use production test account #1, Domain "many-domain-records" for a Domain with 150 CNAME records.

This uses the same Local Storage key as Linodes, Domains, Volumes, and NodeBalancers, so if you select "Show All" on one of those pages, you'll see all of your Domain Records listed as well.
@johnwcallahan johnwcallahan self-assigned this Jan 14, 2020
@jstamis-linode jstamis-linode self-requested a review January 14, 2020 19:42
@Jskobos
Copy link
Contributor

Jskobos commented Jan 14, 2020

I guess I'd say make this a separate page size key, but it's not a huge deal either way.

Copy link
Contributor

@jstamis-linode jstamis-linode left a comment

Choose a reason for hiding this comment

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

I took a look at this with the listed account locally and on the server. I did not see the performance issues described. It is still a good idea to keep something around should this become and issue.

This looks good to me.

@johnwcallahan johnwcallahan merged commit 06b3e5d into linode:develop Jan 16, 2020
@johnwcallahan johnwcallahan deleted the M3-3803-show-all-domain-records branch January 16, 2020 17:35
@johnwcallahan
Copy link
Contributor Author

@Jskobos since we use the same local storage key for all other entities, I left it the same for this one. We should probably discuss using user preferences for these anyway.

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.

3 participants