feat: #1125 Standalone app page #1240

nphivu414 · 2020-05-13T08:48:21Z

Currently, as a user client or developer, I view app details in a modal. These modals have grown too small for their use case so require a standalone app page

…t/1125-standalone-app-page

…lows

duong-se · 2020-05-13T09:19:57Z

packages/marketplace/src/components/pages/developer-app-detail/developer-app-detail.tsx

+        onUninstallSuccess={closeInstallationsModal(setIsInstallationsModalOpen)}
+      />
+
+      <AppRevisionModal


I think should unmount and remove from DOM tree modal isAppRevisionComparisionModalOpen && <AppRevisionModal ... if we don't show it on UI

duong-se · 2020-05-13T09:34:16Z

packages/marketplace/src/components/ui/app-detail/app-content/app-content.tsx

+    speed: 500,
+    variableWidth: true,
+    prevArrow: (
+      // @ts-ignore


you can write like this

const renderSlickButton = () => { return ( <SlickButtonNav> <ChevronLeftIcon /> </SlickButtonNav> ) } ... prevArrow: renderSlickButton(), ...

duong-se · 2020-05-13T09:34:26Z

packages/marketplace/src/components/ui/app-detail/app-content/app-content.tsx

+      </SlickButtonNav>
+    ),
+    nextArrow: (
+      // @ts-ignore


same as above

duong-se · 2020-05-13T09:37:16Z

packages/marketplace/src/components/ui/app-detail/app-content/app-content.tsx

+      <GridItem className="is-8">
+        <HTMLRender className={styles.description} html={description} />
+        <br />
+        {carouselImages.length > 0 && (


I think this component too long pls separate to function renderCarousel

Hi @tanphamhaiduong,
Because this view is going to be changed when building the new UI for standalone app page, refactoring these codes here will means nothing, we will still have to rework anyway

Yeah, we may not even use the carousel. It's fine to leave

duong-se · 2020-05-13T09:37:30Z

packages/marketplace/src/components/ui/app-detail/app-content/app-content.tsx

+        )}
+        <br />
+        <Grid isMultiLine>
+          {scopes.map(item => (


I think this component too long pls separate to function renderScopes

Hi @tanphamhaiduong,
Because this view is going to be changed when building the new UI for standalone app page, refactoring these codes here will means nothing, we will still have to rework anyway

duong-se · 2020-05-13T09:40:01Z

packages/marketplace/src/components/pages/developer-home.tsx

+
+  if (location && location.search) {
+    const pageQueryString = getParamValueFromPath(location.search, 'page')
+    pageNumber = Number(pageQueryString) > 0 ? Number(pageQueryString) : 1


So do we need to check this Number(pageQueryString) > 0 ?

duong-se · 2020-05-13T09:42:26Z

packages/marketplace/src/components/ui/client-app-detail/client-app-install-confirmation.tsx

+        }
+      >
+        <>
+          {scopes.length ? (


I think should move this out because function render too long.

Hi @tanphamhaiduong,
Because this view is going to be changed when building the new UI for standalone app page, refactoring these codes here will means nothing, we will still have to rework anyway

duong-se · 2020-05-13T09:43:50Z

packages/marketplace/src/components/ui/client-app-detail/client-app-install-confirmation.tsx

+          )}
+        </>
+      </Modal>
+      <Modal visible={isSuccessAlertVisible} afterClose={handleSuccessAlertMessageAfterClose(setIsSuccessAlertVisible)}>


I think should un mount if we don't use modal isSuccessAlertVisible && <Modal visible={isSuccessAlertVisible} ...

duong-se · 2020-05-13T09:44:11Z

packages/marketplace/src/components/ui/client-app-detail/client-app-install-confirmation.tsx

+
+  return (
+    <>
+      <Modal


visible && Modal ...

duong-se · 2020-05-13T09:44:50Z

packages/marketplace/src/components/ui/client-app-detail/client-app-uninstall-confirmation.tsx

+        visible={visible}
+        title={`Confirm ${name} installation`}
+        afterClose={closeUninstallConfirmationModal}
+        footerItems={


This func too long pls move to renderFooterItems function

duong-se · 2020-05-13T09:45:21Z

packages/marketplace/src/components/ui/client-app-detail/client-app-uninstall-confirmation.tsx

+
+  return (
+    <>
+      <Modal


unmount if we dont use it

duong-se · 2020-05-13T09:45:27Z

packages/marketplace/src/components/ui/client-app-detail/client-app-uninstall-confirmation.tsx

+      >
+        <>Are you sure you wish to uninstall {name}? This action will uninstall the app for ALL platform users</>
+      </Modal>
+      <Modal visible={isSuccessAlertVisible} afterClose={handleSuccessAlertMessageAfterClose(setIsSuccessAlertVisible)}>


unmount if we dont use it

duong-se · 2020-05-13T09:53:09Z

packages/marketplace/src/components/ui/developer-app-detail/app-revision-modal.tsx

+          visible={isConfirmationModalVisible}
+          title="Please confirm"
+          afterClose={() => setIsConfirmationModalVisible(false)}
+          footerItems={


could you move this to function renderItems

duong-se · 2020-05-13T09:53:21Z

packages/marketplace/src/components/ui/developer-app-detail/app-revision-modal.tsx

+          <p>Are you sure you wish to cancel any pending revisions for this App?</p>
+        </Modal>
+
+        <Modal visible={isDeclinedSuccessfully} afterClose={afterClose}>


isDeclinedSuccessfully && Modal

duong-se · 2020-05-13T09:53:30Z

packages/marketplace/src/components/ui/developer-app-detail/app-revision-modal.tsx

+        ) : (
+          <AppRevisionComparision appDetailData={appDetailState.data} revisionDetailState={appRevisionDetail} />
+        )}
+        <Modal


isConfirmationModalVisible && isConfirmationModalVisible

willmcvay

Some small tweaks but looks great, nice work 💯

willmcvay · 2020-05-13T09:13:52Z

packages/marketplace/src/actions/client.ts

@@ -1,9 +1,17 @@
 import { actionCreator } from '../utils/actions'
 import ActionTypes from '../constants/action-types'
-import { ClientItem, ClientParams } from '../reducers/client'
+import { ClientAppSummary, ClientAppSummaryParams } from '../reducers/client/app-summary'


Small point but @/reducers/...

willmcvay · 2020-05-13T09:16:55Z

packages/marketplace/src/components/pages/client-app-detail/client-app-detail.tsx

+  }
+}
+
+export const renderAppHeaderButtonGroup = (id, installedOn, onInstallConfirmationModal) => {


Can you add definitions to these params please?

...marketplace/src/components/pages/developer-app-detail/__test__/developer-app-detail.test.tsx

willmcvay · 2020-05-13T09:21:27Z

packages/marketplace/src/components/pages/developer-app-detail/developer-app-detail.tsx

+  }
+}
+
+export const renderAppHeaderButtonGroup = (


Can you add definitions to these params please?

willmcvay · 2020-05-13T09:24:06Z

packages/marketplace/src/components/pages/developer-home.tsx

-  const pageNumber = match.params && !isNaN(match.params.page) ? Number(match.params.page) : 1
+  let pageNumber = 1
+
+  if (location && location.search) {


Would be nice if there was a pagination util that gets this detail from the URL where required and returns 1 as the default. Not a big deal but would be nice.

Sorry, I don't really get your idea here, can you explain it a bit more for me?

Just a getPageFromLocation helper that would return the page number from location. It's not important though

willmcvay · 2020-05-13T11:00:21Z

...lace/src/components/ui/client-app-detail/__test__/client-app-uninstall-confirmation.test.tsx

+  renderUninstallConfirmationModalFooter,
+} from '../client-app-uninstall-confirmation'
+
+const mockState = {


And again with the mock state :-)

willmcvay · 2020-05-13T11:02:15Z

packages/marketplace/src/components/ui/client-app-detail/client-app-uninstall-confirmation.tsx

+  }
+}
+
+export const handleSuccessAlertButtonClick = history => {


This history.replace function is used a lot. Feels like we should use a util helper. Also, history is type History :-)

We can't build a util helper for history.replace and call it directly in functions like handleSuccessAlertButtonClick because useHistory hook can only called in a function component

willmcvay · 2020-05-13T11:03:29Z

...ages/marketplace/src/components/ui/developer-app-detail/__test__/app-revision-modal.test.tsx

+import { revisionDetailRequestData } from '@/actions/revision-detail'
+import { developerFetchAppDetail } from '@/actions/developer'
+
+const mockState = {


And again with mock state - mock state module?

willmcvay · 2020-05-13T11:07:22Z

packages/marketplace/src/reducers/client/app-summary.ts

+  error: null,
+}
+
+const appSumarryReducer = (state: ClientAppSummaryState = defaultState, action: Action<any>): ClientAppSummaryState => {


Small typo, summary has 1 r :-)

willmcvay · 2020-05-13T11:10:00Z

packages/marketplace/src/services/apps.ts

+import { fetcher } from '@reapit/elements'
+import { URLS, generateHeader } from '../constants/api'
+
+export interface FetchAppDetailParams {


You define this interface but don't use it!

Also, needs a test I think for this file

FetchAppDetailParams is used in src/actions/client.ts
export const clientFetchAppDetail = actionCreator<FetchAppDetailParams>(ActionTypes.CLIENT_FETCH_APP_DETAIL)

and in src/actions/developer.ts
export const developerFetchAppDetail = actionCreator<FetchAppDetailParams>(ActionTypes.DEVELOPER_FETCH_APP_DETAIL)

I added a test for this file by the way

…t/1125-standalone-app-page

willmcvay

LGTM, nice one 👍

willmcvay · 2020-05-14T06:58:10Z

packages/marketplace/src/components/pages/developer-home.tsx

-  const pageNumber = match.params && !isNaN(match.params.page) ? Number(match.params.page) : 1
+  let pageNumber = 1
+
+  if (location && location.search) {


Just a getPageFromLocation helper that would return the page number from location. It's not important though

willmcvay · 2020-05-14T07:00:05Z

packages/marketplace/src/reducers/__stubs__/app-state.ts

@@ -0,0 +1,219 @@
+import { ReduxState } from '@/types/core'
+
+const appState: ReduxState = {


vuhuucuong

Others look good to me👍

vuhuucuong · 2020-05-14T07:12:26Z

packages/marketplace/src/components/ui/app-detail/app-content/app-content.tsx

+
+export type AppContentProps = {
+  appDetailData: AppDetailModel & {
+    apiKey?: string | undefined


No need | undefined here
apiKey?: string is enough

vuhuucuong · 2020-05-14T07:13:41Z

packages/marketplace/src/components/ui/app-detail/app-header/app-header.tsx

+
+export type AppHeaderProps = {
+  appDetailData: AppDetailModel & {
+    apiKey?: string | undefined


No need |undefined

vuhuucuong · 2020-05-14T07:20:32Z

packages/marketplace/src/components/ui/app-detail/app-header/app-header.tsx

+          <div className={styles.appIconContainer}>
+            <img
+              className="image"
+              src={(appIcon && appIcon.uri) || 'https://bulma.io/images/placeholders/48x48.png'}


appIcon?.uri || 'https://...' for short

vuhuucuong · 2020-05-14T07:24:36Z

packages/marketplace/src/components/ui/developer-app-detail/app-revision-comparision.tsx

+  appScopes: ScopeModel[] | undefined
+  revisionScopes: ScopeModel[] | undefined


appScopes?: ScopeModel[]
revisionScopes?: ScopeModel[]

nphivu414 added 20 commits May 4, 2020 18:35

feat: #1125: Create app details standalone page

4ec9e74

feat: #1125: Update standalone app detail layout

406b6c0

feat: #1125: Update app detail features

07aae78

feat: #1125: Update app detail header action buttons

36c3b30

feat: #1125: Update client app detail standalone page

3975f4f

feat: #1125: Remove redundant logs

2c19976

feat: #1125: Add client app uninstall confirm dialog

5b0d9cb

Merge branch 'master' of ssh://github.com/reapit/foundations into fea…

9649663

…t/1125-standalone-app-page

feat: #1125: Update client app installation flow

ddd314f

feat: #1125: Update new redux flow for client apps

868db84

feat: #1125: Update client detaill app install + uninstall behavior f…

c35dee4

…lows

feat: #1125: Merge master and fix conflicts

863268d

feat: #1125: Update client app detail redux action name

f4917e4

feat: #1125: Update developer app detail behavior flows

64c6709

feat: #1125: Update new redux flow for developer apps

e35560c

feat: #1125: Remove comments & update modal handler param names

59d2133

feat: #1125: Merge master into 1125 & fix conflicts

d500062

feat: #1125: Update tests

49fabe7

feat: #1125: Update tests for components using react-redux hooks

11bfa62

feat: #1125: Merge master into 1125 and fix conflicts

46da4ce

github-actions bot requested review from ant066, phmngocnghia, duong-se, trankhacvy, vuhuucuong and willmcvay May 13, 2020 08:48

github-actions bot assigned nphivu414 May 13, 2020

github-actions bot added elements Relates to Elements components and utilities marketplace Relates to the Marketplace labels May 13, 2020

feat: #1125: Remove unused var

a0f740a

duong-se reviewed May 13, 2020

View reviewed changes

feat: #1125: Update snapshot tests

e2e5db9

github-actions bot requested a review from duong-se May 13, 2020 09:21

duong-se reviewed May 13, 2020

View reviewed changes

feat: #1125: Update codes based on reviews

9b6b4d7

github-actions bot requested a review from duong-se May 13, 2020 10:46

feat: #1125: Update codes based on reviews and fix router snapshot tests

f65c93c

willmcvay requested changes May 13, 2020

View reviewed changes

feat: #1125: Update redux mock store + Fix missing type issues

5e83048

github-actions bot requested a review from willmcvay May 14, 2020 04:15

nphivu414 added 3 commits May 14, 2020 11:20

feat: #1125: Update uninstall app modal rendeing flow

acf7726

feat: #1125: Fix lint issues

eb08903

Merge branch 'master' of ssh://github.com/reapit/foundations into fea…

0a61d4c

…t/1125-standalone-app-page

willmcvay approved these changes May 14, 2020

View reviewed changes

vuhuucuong approved these changes May 14, 2020

View reviewed changes

nphivu414 merged commit 12cc29d into master May 14, 2020

nphivu414 deleted the feat/1125-standalone-app-page branch May 14, 2020 07:48

		@@ -0,0 +1,219 @@
		import { ReduxState } from '@/types/core'

		const appState: ReduxState = {

		appScopes: ScopeModel[] \| undefined
		revisionScopes: ScopeModel[] \| undefined

feat: #1125 Standalone app page #1240

feat: #1125 Standalone app page #1240

Conversation

nphivu414 commented May 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willmcvay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willmcvay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vuhuucuong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nphivu414 commented May 13, 2020 •

edited

Loading