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

Paragon table deprecation migration to DataTable #200

Conversation

abdullahwaheed
Copy link
Contributor

Ticket

Migrate off deprecated Paragon components

What has changed

Removed deprecated Table component of paragon and updated it with DataTable

References

Paragon Table
Paragon DataTable

image

@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Patch and project coverage have no change.

Comparison is base (d0d4ede) 57.04% compared to head (ffa4326) 57.04%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #200   +/-   ##
=======================================
  Coverage   57.04%   57.04%           
=======================================
  Files          29       29           
  Lines         305      305           
  Branches       64       64           
=======================================
  Hits          174      174           
  Misses        126      126           
  Partials        5        5           
Impacted Files Coverage Δ
src/order-history/OrderHistoryPage.jsx 76.74% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@abdullahwaheed
Copy link
Contributor Author

based on #191

@mphilbrick211
Copy link

@colinbrash please review / merge. Thank you!

@mphilbrick211
Copy link

@arbrandes would you mind taking a look at this? It's been open for a bit - seeing if it can get reviewed.

@arbrandes
Copy link
Contributor

@mphilbrick211, prior to the Olive release I don't think I'll have time to test this properly, but I'll add it to my list!

@arbrandes
Copy link
Contributor

@pshiu, another ecommerce one, if you happen to have some time. Thanks!

@arbrandes arbrandes requested a review from pshiu January 23, 2023 19:05
Copy link
Contributor

@julianajlk julianajlk left a comment

Choose a reason for hiding this comment

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

Thanks for including the feedback from #191!

I see that a pagination was added with Prev/Next buttons, in the screenshot there's only 1 course with 2 pages, is that accurate with what prod will have? I believe this MFE uses pageSize as a determinant for how many pages it should have, is this a bug?

If we're using the Prev/Next (vs. Showing 1 of 2), could you adjust the padding? It looks like the buttons are overlaid on top of the table border.

…to abdullahwaheed/paragon-table-deprecations
@abdullahwaheed
Copy link
Contributor Author

@julianajlk that's an old PR but i think i was testing pagination with 2 entries by manually setting up limit to 1 item per page. Let me test it again with some more data

@abdullahwaheed
Copy link
Contributor Author

Tested again, screenshot was added to show the change with dummy data and page size was set to 1 explicitly to show pagination. Also fixed the design of pagination footer. Its not ready to review and merge

image

@abdullahwaheed abdullahwaheed requested review from julianajlk and removed request for colinbrash and pshiu February 3, 2023 15:42
@julianajlk
Copy link
Contributor

Tested again, screenshot was added to show the change with dummy data and page size was set to 1 explicitly to show pagination. Also fixed the design of pagination footer. Its not ready to review and merge

image

Thanks for testing! How will it look like with multiple pages?

@abdullahwaheed
Copy link
Contributor Author

abdullahwaheed commented Feb 6, 2023

Thanks for testing! How will it look like with multiple pages?

It looks like this

image

Copy link
Contributor

@julianajlk julianajlk left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, thanks for including my feedback!

@abdullahwaheed
Copy link
Contributor Author

@julianajlk any updates on this?
its approved and awaiting merge. we cannot merge this PR

@arbrandes
Copy link
Contributor

I'm technically not allowed to merge this, either. @abdullahwaheed, do you mind resolving conflicts? Then I'll try and get the owning team to merge it.

@abdullahwaheed
Copy link
Contributor Author

I'm technically not allowed to merge this, either. @abdullahwaheed, do you mind resolving conflicts? Then I'll try and get the owning team to merge it.

I'll update it

…to abdullahwaheed/paragon-table-deprecations
Copy link
Contributor

@zubair-ce07 zubair-ce07 left a comment

Choose a reason for hiding this comment

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

LGTM ! Tested locally and it's working fine.

@zubair-ce07 zubair-ce07 merged commit 74cc3da into openedx:master May 25, 2023
grmartin added a commit that referenced this pull request Aug 29, 2023
* Update standard workflow files. (#263)

* build: Create a missing workflow file `self-assign-issue.yml`.

The .github/workflows/self-assign-issue.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Create a missing workflow file `add-remove-label-on-comment.yml`.

The .github/workflows/add-remove-label-on-comment.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Update a workflow file `add-depr-ticket-to-depr-board.yml`.

The .github/workflows/add-depr-ticket-to-depr-board.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* chore(i18n): add more languages (#261)

* chore(i18n): add more languages

* chore(i18n): Pylint fixes

* feat: use `atlas` in `make pull_translations` (#278)

Changes
-------
 - Bump frontend-platform to bring `intl-imports.js` script
 - Move all i18n imports into `src/i18n/index.js` so `intl-imports.js` can
   override it with latest translations
 - Add `atlas` into `make pull_translations` when `OPENEDX_ATLAS_PULL`
   environment variable is set.

Refs: [FC-0012 project](https://openedx.atlassian.net/l/cp/XGS0iCcQ) implementing Translation Infrastructure OEP-58.

* feat: upgraded to node v18, added .nvmrc and updated workflows (#274)

* feat: upgraded to node v18, added .nvmrc and updated workflows

* build: update frontrnf-build

* build: upgrade pkgs

* build: update @edx/browserslist-config to the latest

* refactor: update packages

* feat: add subscriptions section to orders page (#289)

* feat: design changes for subscriptions (#276)

* feat: api integration for subscription related changes (#282)

* feat: add manage subscriptions url and update subscription upsell (#288)

* test: add more tests for subscriptions

---------

Co-authored-by: Nawfal Ahmed <111358247+NawfalAhmed@users.noreply.github.com>
Co-authored-by: Nawfal Ahmed <nawfal.ahmed@arbisoft.com>

* fix: fix getting b2c subscriptions flag for stage (#290)

* refactor: update components and utils folder structure

* feat: improve subscription section and add basic alerts

* fix: revert change to container (#292)

* Automate Browserlist DB Update (#221)

* feat: added cron github action to auto update brwoserlist DB periodically

* refactor: used a shared script to update broswerslist DB, create PR and automerge it

* Paragon table deprecation migration to DataTable (#200)

* refactor: changed derecated table component to datatable

* refactor: fixed warning of missing props

* refactor: changed DataTable to DataTable.Table as suggested in code review

* refactor: used DataTable.Table directly to remove count

* fix: fixed pagination styling issue

* fix: fixed snapshot test

* PON-251: Update Existing Error Handling (#294)

* feat: update order history error flows

* refactor: remove unused AsyncActionType, favour saga routines over this

* fix: add sr message to loading, fix order history loading

* test: add more tests for coverage

* feat: update error UX for manage subscriptions (#299)

* chore(i18n): update translations (#301)

Co-authored-by: Jenkins <sre+jenkins@edx.org>

* feat: handle subscription api statuscode flag (#304)

* feat: add translations for subscription state badges (#305)

* fix: fix unmarked translations (#306)

* chore(i18n): update translations

* chore(i18n): update translations (#308)

Co-authored-by: Jenkins <sre+jenkins@edx.org>

* feat: add subscriptions marketing url (#309)

* docs: update readme to reflect recent subscription changes (#312)

* feat: move subscription upsell values behind env (#314)

* feat: use react-testing-library for detailed unit tests (#316)

* feat: use react-testing-library for tests

* feat: update tests for Subscriptions

* feat: update tests for NotFoundPage

* feat: update tests for ManageSubscriptions

* feat: update tests for OrdersandSubscriptions

* chore: added CODEOWNERS file to enable branch protection policy (REV-3441) (#283)

* chore: added CODEOWNERS file to enable branch protection policy

* fix: updated @edx/revenue-squad team tag

* fix: updated @openedx/revenue-squad team tag

* chore: added self check for CODEOWNERS file and updated file path to /src

* feat: disable subscriptions for local (#318)

* Removed subscription section from Readme file (#322)

* fix: removed subscription section and subs screenshot from readme file

* feat: update react & react-dom to v17 (#302)

* feat: update react & react-dom to v17

* build: update edx pkgs

* build: update lock file

---------

Co-authored-by: Feanil Patel <feanil@tcril.org>
Co-authored-by: Yoiber <105317298+Yoiber071@users.noreply.github.com>
Co-authored-by: Omar Al-Ithawi <i@omardo.com>
Co-authored-by: Mashal Malik <107556986+Mashal-m@users.noreply.github.com>
Co-authored-by: Shafqat Farhan <shafqat.farhan@arbisoft.com>
Co-authored-by: Nawfal Ahmed <111358247+NawfalAhmed@users.noreply.github.com>
Co-authored-by: Nawfal Ahmed <nawfal.ahmed@arbisoft.com>
Co-authored-by: Muhammad Abdullah Waheed <42172960+abdullahwaheed@users.noreply.github.com>
Co-authored-by: edx-transifex-bot <edx-transifex-bot@users.noreply.github.com>
Co-authored-by: Jenkins <sre+jenkins@edx.org>
Co-authored-by: Muhammad Mattiullah <50130127+MuhammadMattiullah@users.noreply.github.com>
Co-authored-by: Shahroz Ahmad <97090106+ishahroz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants