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

feat: #1126 Route to the new standalone app page #1368

Merged
merged 18 commits into from
Jun 2, 2020

Conversation

nphivu414
Copy link
Contributor

@nphivu414 nphivu414 commented May 27, 2020

Pull request checklist

Does this close any currently open issues?

fixes: #

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (yarn build) was run locally and any changes were pushed
  • Lint (yarn lint) has passed locally and any fixes were made for failures
  • Test (yarn test) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

  • We loaded app detail as a modal
  • Standalone app page doesn't have "Back To Apps" button at the bottom of the page

Issue Number: #1126

What is the new behavior?

  • We should now route to the appropriate standalone app page.
  • There should be a 'back to apps' link at the bottom of the page like the apps edit page

Does this introduce a breaking change?

  • Yes
  • No

Other information

Please do not merge this ticket. I'll merge this ticket myself after tickets #1206 #1225 are completed and passed QA

Before:
1126_0
1126

After:
1126_1

Copy link
Contributor

@duong-se duong-se left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@vuhuucuong vuhuucuong left a comment

Choose a reason for hiding this comment

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

LGTM

@nphivu414 nphivu414 added the do not merge Branch should not be merged into main workflow in current state label May 29, 2020
@duong-se duong-se removed the do not merge Branch should not be merged into main workflow in current state label Jun 2, 2020
@github-actions github-actions bot requested review from duong-se and vuhuucuong June 2, 2020 05:50
@github-actions github-actions bot added the aml-checklist Relates to Anti-money laundering app label Jun 2, 2020
@nphivu414 nphivu414 merged commit c34cfd5 into master Jun 2, 2020
@nphivu414 nphivu414 deleted the feat/1126-route-to-the-new-standalone--app-page branch June 2, 2020 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aml-checklist Relates to Anti-money laundering app marketplace Relates to the Marketplace web-components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants