PROD-3109 PROD-3106 PROD-2708 PROD-2640 -> Fix issues in self service -> dev #413
Conversation
…s not match the provided text
…the WM challenge specification and challenge details page
…k from the initial draft
brooketopcoder
left a comment
brooketopcoder
left a comment
There was a problem hiding this comment.
@asadath1395 This looks great! Nice work.
I just have a couple super minor comments. Thanks!
| margin-top: $space-xxl; | ||
| } | ||
|
|
||
| .body-medium-bold { |
There was a problem hiding this comment.
Rather than completely redefining a global style, body-medium-bold, can we find the global style that actually matches the design? Figma designs have the name of the font we should use.
The only things we should be overriding for global fonts are related to padding/layout.
| margin-bottom: $space-xxl; | ||
| } | ||
|
|
||
| .body-medium-bold { |
| svg.card-row-icon { | ||
| height: 14px; | ||
| width: 14px; | ||
| height: 24px; |
There was a problem hiding this comment.
let's use the _icons.mixins to size icons.
| margin-right: 6px; | ||
|
|
||
| @include ltemd { | ||
| height: 16px; |
| <span className={cn(styles['card-row-col'], styles['center'])}> | ||
| { row.valueIcon ? | ||
| <IconOutline.CheckIcon width={18} height={16} /> : | ||
| <IconOutline.CheckIcon width={28} height={20} /> : |
There was a problem hiding this comment.
let's use the mixins in the stylesheets to size icons.
src-ts/utils/bug-hunt/index.ts
Outdated
| @@ -0,0 +1,16 @@ | |||
| import { currencyFormat } from '../../../src/utils' | |||
There was a problem hiding this comment.
I know this is a confusing naming convention, but /src-ts/utils in this case refers specifically to the utilities in the header, like Account Settings. See the README.
Anything related to bughunt and the work tool needs to be located w/in the /src-ts/tools/work directory. Perhaps, this would go in the src-ts/tools/work/work-lib/work-provider/work-functions/work.functions.ts?
Also, we should not be importing anything from the /src directory into the /src-ts directory unless we absolutely have to. And then there should be comment explaining why it has to be imported.
In this case, please use the textFormatMoneyLocaleString method from the /src-ts/lib to format money.
|
@brooketopcoder Could you please re-review this? Thanks |
brooketopcoder
left a comment
brooketopcoder
left a comment
There was a problem hiding this comment.
Excellent--approved.
|
@asadath1395 Please do not merge this until after the production deployment this afternoon. |
Fixes the following issues in self service project