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

PON-106&80: Subscription related design changes #276

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

NawfalAhmed
Copy link
Member

@NawfalAhmed NawfalAhmed commented Apr 20, 2023

Description

Subscription related design changes

Supporting information

Relevant Jira Issues PON-106, PON-80

Screenshots

image

image

@NawfalAhmed NawfalAhmed marked this pull request as draft April 20, 2023 08:54
@NawfalAhmed NawfalAhmed self-assigned this Apr 20, 2023
@NawfalAhmed NawfalAhmed force-pushed the nahmed/pon-106/design-orders-and-subs branch 5 times, most recently from 69cb395 to e3b5666 Compare April 20, 2023 10:19
src/order-and-subscriptions/OrderAndSubscriptionsPage.jsx Outdated Show resolved Hide resolved
src/order-and-subscriptions/_style.scss Outdated Show resolved Hide resolved
src/order-and-subscriptions/index.js Outdated Show resolved Hide resolved
src/order-history/OrderHistoryPage.jsx Show resolved Hide resolved
src/order-and-subscriptions/OrderAndSubscriptionsPage.jsx Outdated Show resolved Hide resolved
src/subscriptions/SubscriptionScrollView.jsx Outdated Show resolved Hide resolved
src/subscriptions/SubscriptionScrollView.jsx Outdated Show resolved Hide resolved
@NawfalAhmed NawfalAhmed force-pushed the nahmed/pon-106/design-orders-and-subs branch from e3b5666 to 3e0a498 Compare April 20, 2023 11:12
@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Patch coverage: 80.55% and project coverage change: +9.27 🎉

Comparison is base (1ef0847) 50.00% compared to head (cba866e) 59.27%.

Additional details and impacted files
@@                 Coverage Diff                  @@
##           ponderosa-master     #276      +/-   ##
====================================================
+ Coverage             50.00%   59.27%   +9.27%     
====================================================
  Files                    15       19       +4     
  Lines                   164      194      +30     
  Branches                 28       32       +4     
====================================================
+ Hits                     82      115      +33     
+ Misses                   77       75       -2     
+ Partials                  5        4       -1     
Impacted Files Coverage Δ
src/index.jsx 0.00% <ø> (ø)
src/subscriptions/SubscriptionUpsell.jsx 50.00% <50.00%> (ø)
...s-and-subscriptions/OrdersAndSubscriptionsPage.jsx 70.00% <70.00%> (ø)
src/subscriptions/Subscriptions.jsx 83.33% <83.33%> (ø)
src/order-history/OrderHistoryPage.jsx 79.06% <87.50%> (+1.02%) ⬆️
src/subscriptions/SubscriptionScrollView.jsx 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

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.

Great work! I see you'll be adding more tests.
One question from the screenshot, is there a border line at the bottom of the last subscription Card?

src/index.jsx Outdated
@@ -17,7 +17,7 @@ import { messages as paragonMessages } from '@edx/paragon';
import messages from './i18n';
import configureStore from './store';
import NotFoundPage from './components/NotFoundPage';
import { ConnectedOrderHistoryPage } from './order-history';
import { OrderAndSubscriptionsPage } from './order-and-subscriptions';
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe name it OrdersAndSubscriptionsPage (plural in orders)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure,

is there a border line at the bottom of the last subscription Card?

That is the shadow made by the scrollable when there are more than 3 cards, the cards will scroll

src/index.scss Outdated
@@ -11,6 +11,7 @@ $fa-font-path: "~font-awesome/fonts";
@import "~@edx/frontend-component-footer/dist/footer";

@import "./order-history/style";
@import "./order-and-subscriptions/style";
Copy link
Contributor

Choose a reason for hiding this comment

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

Same case for plural orders here?

src/order-and-subscriptions/OrderAndSubscriptionsPage.jsx Outdated Show resolved Hide resolved
src/order-and-subscriptions/OrderAndSubscriptionsPage.jsx Outdated Show resolved Hide resolved
@NawfalAhmed NawfalAhmed force-pushed the nahmed/pon-106/design-orders-and-subs branch 4 times, most recently from 32cfae1 to edd2719 Compare April 25, 2023 11:49
@NawfalAhmed NawfalAhmed marked this pull request as ready for review April 25, 2023 11:50
@NawfalAhmed NawfalAhmed force-pushed the nahmed/pon-106/design-orders-and-subs branch from edd2719 to cba866e Compare April 25, 2023 11:56
@NawfalAhmed NawfalAhmed merged commit c90589a into ponderosa-master Apr 25, 2023
5 checks passed
@NawfalAhmed NawfalAhmed deleted the nahmed/pon-106/design-orders-and-subs branch April 25, 2023 12:20
NawfalAhmed added a commit that referenced this pull request May 17, 2023
* 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>
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants