Skip to content

Conversation

@asadath1395
Copy link
Contributor

@asadath1395 asadath1395 commented Oct 20, 2022

Fixes the following issues

  1. PROD-2643 Wrong icon shown for bug hunt self-service
  2. PROD-2619 Fix the back button on the first page is opening the previously visited page, not the Start Work page
  3. PROD-2620 Fix when opening bug hunt drafts after opening another work type's draft, the bug hunt drafts cannot be opened
  4. PROD-2760 Fix issues with Timeline status in Bug hunt work
  5. PROD-2644 Fix unable to proceed to pay if user submits the contact support form

@brooketopcoder brooketopcoder changed the title Fix the issues in self-service PROD-2643 PROD-2619 PROD-2620 PROD-2760 Fix the issues in self-service -> dev Oct 20, 2022
@brooketopcoder
Copy link
Contributor

I updated the PR name to match our preferred convention. For future ref, I added more info to the README: https://github.com/topcoder-platform/platform-ui#pull-requests

Copy link
Contributor

@brooketopcoder brooketopcoder left a comment

Choose a reason for hiding this comment

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

Excellent work @asadath1395 !

All my comments are super nit-picky. Let me know if you have any questions, etc.

@import '../styles/includes';

.iconWrapper {
height: 48px;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's either use use the icon size mixins for the wrapper or the $space-mxx var

@mixin icon-mxx;
or
height: $space-mxx;
width: $space-mxx;

width: 48px;
margin: 0 $space-lg $space-lg $space-lg;
background: $tc-grad12;
border-radius: 24px;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use the space var here too

BugHuntFormConfig.buttons.primaryGroup[1].onClick = () => { setAction('submit') }
if (BugHuntFormConfig.buttons.secondaryGroup) {
BugHuntFormConfig.buttons.secondaryGroup[0].onClick = () => { navigate(-1) }
BugHuntFormConfig.buttons.secondaryGroup[0].onClick = () => { navigate('/self-service/wizard') }
Copy link
Contributor

Choose a reason for hiding this comment

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

All routes should be defined in work.routes.tsx

@@ -0,0 +1,17 @@
import classNames from 'classnames'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this directory to be nested in the svgs directory and rename the component file IconWrapper.tsx.

import { FC } from 'react'

import { IconOutline, textFormatMoneyLocaleString, Tooltip } from '../../../lib'
import IconWrapper from '../../../lib/icon-wrapper'
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add a barrel file to the icon-wrapper directory and export the component from the parent /lib directory:

src-ts/lib/svgs/icon-wrapper/index.ts

export { default as IconWrapper } from './icon-wrapper'

src-ts/lib/svgs/index.ts

export * from './icon-wrapper'

src-ts/tools/work/work-service-price/WorkServicePrice.tsx

import { IconOutline, IconWrapper, textFormatMoneyLocaleString, Tooltip } from '../../../lib'

import { FC, SVGProps } from 'react'

import { IconOutline } from '../../../../lib'
import IconWrapper from '../../../../lib/icon-wrapper'
Copy link
Contributor

Choose a reason for hiding this comment

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

See prior comment about exporting/importing from the /lib/index.ts

WorkTypeCategoryDataIcon,
WorkTypeCategoryDesignIcon,
WorkTypeCategoryUnknownIcon,
Work,
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird--not sure why it changed the tabbing from 4 spaces to 2 spaces here, but 4 was correct.

Icon = (
<IconWrapper
className={styles['qa-icon']}
icon={<IconOutline.BadgeCheckIcon width={48} height={48} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

The Icon wrapper already assigns a height/width to the svg element using a css mixin, which is the preferred method, so there is no need to specify them here.

import { INTAKE_FORM_ROUTES as DATA_ADVISORY_INTAKE_FORM_ROUTES } from "./constants/products/DataAdvisory";
import { INTAKE_FORM_ROUTES as WEBSITE_DESIGN_INTAKE_FORM_ROUTES } from "./constants/products/WebsiteDesign";
import { INTAKE_FORM_ROUTES as WEBSITE_DESIGN_LEGACY_INTAKE_FORM_ROUTES } from "./constants/products/WebsiteDesignLegacy";
import { BUG_HUNT_ROUTE } from "./constants/products/BugHunt";
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything to do w/routes should be defined in src-ts/tools/work/work.routes.tsx.

@asadath1395 asadath1395 changed the title PROD-2643 PROD-2619 PROD-2620 PROD-2760 Fix the issues in self-service -> dev PROD-2643 PROD-2619 PROD-2620 PROD-2760 PROD-2644 Fix the issues in self-service -> dev Oct 21, 2022
@asadath1395
Copy link
Contributor Author

@brooketopcoder Appreciate you for taking the time to review this PR. I will resolve all PR comments. Fyi i have added one more bug fix to this PR as its all part of the challenge. Please review

@asadath1395
Copy link
Contributor Author

@brooketopcoder I have addressed all the comments. Please re-review. Thanks

Copy link
Contributor

@brooketopcoder brooketopcoder left a comment

Choose a reason for hiding this comment

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

@asadath1395 Thanks for addressing all my comments. I have 1 more question...

@nikolay83 nikolay83 merged commit 3ff6aed into dev Oct 21, 2022
@asadath1395 asadath1395 removed their assignment Nov 11, 2023
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.

4 participants