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

Remote Data Paradigm #126

Merged

Conversation

evandana
Copy link
Contributor

@evandana evandana commented May 3, 2017

Summary

Allows grid to accommodate server-centric data source

Example

  • Given: 10,000 alarms in a system
  • UI would request one page's worth of data, 10 records
  • UI would be provided with meta data such as total number of records (10,000)
  • UI can display one page while generating pagination UI for all 10,000

Technical Debt

  • demo.html page been added, but could use review (separate effort from this PR)

Technical Notes

<px-data-table 
            data-remote="true"
            enable-column-resize="true"
            filterable$={{_filterable}} 
            first-item-index="{{_firstItemIndex}}"
            greedy-height-with-scroll="true"
            id="alarmGrid" 
            page-size="{{_pageSize}}" 
            show-column-chooser="true" 
            table-data="[[_alarms]]" 
            table-rows="true" 
            total-entries="{{_filteredCount.filtered}}"
            >
</px-data-table>

@mdwragg
Copy link
Contributor

mdwragg commented Jun 2, 2017

Yeah, I just checked it out and that appears to be a regression @evandana.

@mdwragg
Copy link
Contributor

mdwragg commented Jun 2, 2017

Sorry, should've made that clearer, the right border disappearance appears to be a regression only in this PR. In master this does not occur.

tina513 and others added 2 commits June 16, 2017 14:25
# Fixes Defect Rendering Right Border

- Fixed by adjusting structure of HTML to match master branch (two divs had been accidentally collapsed into one)
- Includes updates from master branch
@mdwragg
Copy link
Contributor

mdwragg commented Jun 20, 2017

Hey @evandana - great fixes, thanks! Sadly I can't run the unit tests on IE and I got a failing test on Edge.

For IE, I think we've got too much running on one page, I suggest you pull the 'remote' fixtures and tests out to a separate suite and fixture (see the px-data-table-cell-fixture.html and associated test for inspiration.

Thanks!

@mdwragg
Copy link
Contributor

mdwragg commented Jul 5, 2017

Hi @evandana - I split the tests up (https://github.com/PredixDev/px-data-table/tree/remote-pr) and copied the changes out to a branch on this repo (so I could run the whole suite of tests against Sauce Labs in our CI environment).

Sadly the tests still fail in IE... We need them working in IE before we can accept the change. I'll leave that branch around for a while so you can see the pattern for splitting the tests up. Thanks!

@evandana
Copy link
Contributor Author

evandana commented Jul 5, 2017 via email

@mdwragg
Copy link
Contributor

mdwragg commented Jul 5, 2017

Good point.... We either use the free VMs from https://modern.ie (https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/) or the 'manual test session' option on SauceLabs (there you can pick an IE VM that gives you 10 minutes of use).

To access a local running version of your tests (on your mac) do; wct -p from one terminal window, then in another terminal window using ngrok (link below), do ngrok http 2000 - You'll then get URL to your tests that you can access from anywhere (either a local VM or SauceLabs VM) by going to http://<ngrok url>/components/px-data-table/generated-index.html.

https://ngrok.com/

@mdwragg mdwragg merged commit afac11e into predixdesignsystem:master Oct 16, 2017
@mdwragg
Copy link
Contributor

mdwragg commented Oct 16, 2017

💯 🐐 👍 🥇

Thanks!

mdwragg added a commit that referenced this pull request Oct 16, 2017
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.

5 participants