feat: add alert ds component #1781

kalashshah · 2024-07-30T07:56:03Z

Pull Request Template

Ticket Number

1759

Description

Problem/Feature:

Adds the new alert component to the Design System

Type of Change

New feature

Checklist

Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
- Quick PR label added
Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
- DO NOT Merge PR label added

Frontend Guidelines

Followed frontend guidelines as per the Guidelines Notion Doc

Build & Testing

No errors in the build terminal
Engineer has tested the changes on their local environment
Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

Review & Approvals

Self-review completed
Code review by at least one other engineer
Documentation updates if applicable

Notes

github-actions · 2024-07-30T07:56:21Z

In the StyledAlert styled component, the interpolation for border, background-color, color properties should be wrapped inside var. Like --${backgroundColor} instead of var(--${backgroundColor}).
In the RightContainer styled component, the gap property should be written like gap: var(--spacing-xs, 12px);.
There is a typo in export { ToggleSwitch } from './toggleSwtich';, it should be export { ToggleSwitch } from './toggleSwitch';.
In the alertVariants object, the TickCircleFilled icon should be imported as TickCircleFilled instead of TickCircleFilled,.
In the alertSemantics object, the key 'radio' should be changed to 'radio-button' to match the semanticKeys object.
The textSemantics import in the file is duplicated. It is imported in both ../semantics/semantics.text and ./semantics.text. You should remove one of the imports to avoid redundancy.

After the corrections, the code looks good.

github-actions · 2024-07-30T08:01:29Z

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-07-31 07:08 UTC

rohitmalhotra1420 · 2024-07-30T09:36:15Z

@mishramonalisha76 please review the PR.

mishramonalisha76 · 2024-07-30T12:46:27Z

src/blocks/alert/Alert.tsx

+import { getTextVariantStyles } from 'blocks/Blocks.utils';
+
+export type AlertProps = {
+  /* Child react nodes rendered by Box */


Suggested change

/* Child react nodes rendered by Box */

/* Child react nodes rendered by Alert */

mishramonalisha76 · 2024-07-30T12:48:19Z

src/blocks/alert/Alert.tsx

+  retryable?: boolean;
+  /* Retry function to be called on retry button click */
+  onRetry?: () => void;


this can be a prop like action?: ReactNode; and will a custom react node .
So instead of retryable, onRetry and retryText it will one custom action component

According to the design, we'll not having custom components here, will only require a change of text message later on if required. Let's remove retryable and closeable and use the functions. Will reduce number of props being passed.

@kalashshah let's do actionText, onAction and onClose.

cc: @mishramonalisha76

Done, please check

mishramonalisha76 · 2024-07-30T12:50:55Z

src/blocks/alert/Alert.tsx

+  border-radius: var(--radius-sm);
+  justify-content: center;
+  white-space: nowrap;
+  padding: 12px;


Suggested change

padding: 12px;

padding: var(--spacing-xs);

mishramonalisha76 · 2024-07-30T12:53:13Z

src/blocks/alert/Alert.utils.ts

+    icon: TickCircleFilled,
+    iconColor: 'components-alert-icon-success',
+    borderColor: 'components-alert-stroke-success',
+    bgColor: 'components-alert-background-success',
+    ctaColor: 'components-alert-text-cta-success',
+  },
+  warning: {
+    icon: WarningCircleFilled,
+    iconColor: 'components-alert-icon-warning',
+    borderColor: 'components-alert-stroke-warning',
+    bgColor: 'components-alert-background-warning',
+    ctaColor: 'components-alert-text-cta-warning',


Check text area component for the color implementation for different variants. This constant isnt needed, it can be done in a much optimised way

This is similar to how we have implemented alertModalIcon in the AlertModal.

mishramonalisha76 · 2024-07-30T12:53:40Z

src/blocks/alert/Alert.tsx

+  white-space: nowrap;
+  padding: 12px;
+  justify-content: space-between;
+  border: 1px solid var(--${({ borderColor }) => borderColor});


1px can be var(--border-sm)

mishramonalisha76 · 2024-07-30T12:54:20Z

src/blocks/alert/Alert.tsx

+  description?: string;
+} & TransformedHTMLAttributes<HTMLDivElement>;
+
+const StyledAlert = styled.div<AlertProps & { iconColor: ThemeColors; borderColor: ThemeColors; bgColor: ThemeColors }>`


instead of the iconColor, borderColor and bgColor pass the variant and do it like it is done in TextArea component

mishramonalisha76 · 2024-07-30T12:55:56Z

src/blocks/alert/Alert.tsx

+`;
+
+const Heading = styled.p`
+  ${() => getTextVariantStyles('h5-semibold', 'components-alert-text-default')}


Add font sizes , weight, style and line height for the texts

Please check the implementation of the getTextVariantStyles, it will appropriately add the required styles to the text.

mishramonalisha76 · 2024-07-30T12:56:25Z

src/blocks/alert/Alert.tsx

+  onRetry,
+  retryable = true,
+  retryText = 'Try Again',


not needed can be replaced by action prop

mishramonalisha76 · 2024-07-30T12:56:45Z

src/blocks/alert/Alert.tsx

+  variant = 'info',
+  ...props
+}) => {
+  const { icon: Icon, iconColor, borderColor, bgColor, ctaColor } = alertVariants[variant];


this is not needed

mishramonalisha76 · 2024-07-30T12:58:13Z

src/blocks/alert/Alert.tsx

+          <StyledLink
+            ctaColor={ctaColor}
+            onClick={onRetry}
+          >
+            {retryText}
+          </StyledLink>


replace with action component, even ctaLink parameter isnot needed

github-actions · 2024-07-31T04:55:26Z

The code provided seems to be well-structured and no obvious mistakes or typos were found. The logic also appears to be sound.

All looks good.

github-actions · 2024-07-31T07:02:45Z

I found some issues in the code:

In the Alert component file, there is a typo in the import statement for HoverableSVG. It should be HoverableSVGProps instead of just HoverableSVG.
The Alert component is exporting AlertVariant twice, first as an alias of the interface and then as a type. This could lead to confusion and should be cleaned up.
There are unnecessary exports in the file that could be cleaned up to improve readability.
In the second part of the file, there is a mix-up of imports and exports that seems to be copy-pasted in error.

Overall, the code needs some cleaning up to improve its readability and maintainability.

All looks good.

* feat: add alert ds component * chore: update alert ds acc to review * chore: update component acc to review

* added loading colour * add icon * add components * update styles * update mobile view * update titles * change button labels * update multiplier * update useDailyRewards hook * update unlock profile modal changes * update usedailyrewards hook * fix isLoading onclick * update locked status/ rewards ui * update new users status * add skeleton loader * comment out stake push activities * update skeleton loader * update locked icon * format text * feat: add alert ds component (#1781) * feat: add alert ds component * chore: update alert ds acc to review * chore: update component acc to review * update alert for error messages * update indexes * Update Dashboard.tsx * remove unusued * fixed review comments * Update Button.tsx * update completed state * fixed the spinner and button * fixed the spinner and button * Added spinner component (#1783) * added spinner component * added review changes * added correct variants * added the review comments * added the review comments * Add progress bar to ds (#1787) * feat: add progress bar to ds * chore: update progress bar according to review * add loading state * update locked status * update isLocked * update loading state * remove disabled state from button * update locked divider * update error message * fix icons and illustrations * update naming and imports * update reward activites hook * update type * add types * resolve conments * update box * update text changes * update text * update points real time * update mutiplier conditions in bonus * update gap * update padding referal section * adjust daily section ui * update stake array * update stake api * update rewards verification hook * fix: yield farming layouts and resize buttons to take full width (#1793) * Add support for erc1155 (#1794) * chore: add support for erc1155 * chore: update uiweb version to include erc1155 support * Replaced older tab implementation in trending channels with the new tab block component (#1800) * added * moved function to utils * tab changes * reverted rewards tab change * Update Dashboard.utils.ts * Opt-In Dropdown for dashboard (#1802) * Opt-In Dropdown for dashboard * Fixed the comments and issues * Fixed the context issue * DS-Dapp Notification Component (#1801) * add in-app semantics and block files * update notification component * add localstorage identifier * update library * add duration option * resolve comments * update comment fixes * fix comments * testing notification * update notification component --------- Co-authored-by: rohitmalhotra1420 <[email protected]> * DApp-1804 tag component added in blocks (#1805) * tag in progress * implemented tag component * Tag component implemented in blocks * update hook and export component * update code * add componentdid mount flow * update mount logic * fix stake font size * fix conflicts * Fixed create channel ui , fixed message for unlock profile, removed governance from sidebar (#1817) * fixed minor ui issues * fixed minor ui issues * fixed minor ui issues * fixed select text color * fixed select text color * fixed select text color * fixed chain issue * fixed send notif delegate issue (#1822) * fixed send notif delegate issue * Fixed the send notification button on the navigation for delegatee list * added fixes for alias * added fixes for alias * fixed review comment --------- Co-authored-by: abhishek-01k <[email protected]> * DApp-1799 blocks/table (#1824) * table in progress * in progress * in progress * table complete * fixed header route + reverted changes * added alignment support * set debug to false * fix comments * export type and fix issues * update rewards notification * fix loading state * add activit days * update locked status * updates test whitelist file for nft claims * fixed smart contract for alpha nft * update reset time * refactor code * update fixes * stake push * update types * update yarn.lock * update icons * update icons batch 2 * sort stake section * add local storage key * update hasEnded and has text * fixed channel notifications (#1852) * added the new monotone icons (#1857) * added support for icons in tag components (#1853) * update stat in yield farming data store (#1859) * update refetch leaderboard * fix double text issue - media query * update icon * hide notifs * update font sizes * fix invalid verification proof issue * remove console * update success flow * fixes done * removed the png file * lock file fixed * remove isActive Account in auth hook --------- Co-authored-by: Monalisha Mishra <[email protected]> Co-authored-by: rohitmalhotra1420 <[email protected]> Co-authored-by: Kalash Shah <[email protected]> Co-authored-by: Monalisha Mishra <[email protected]> Co-authored-by: Abhishek <[email protected]> Co-authored-by: abhishek-01k <[email protected]>

feat: add alert ds component

df2daf6

kalashshah requested a review from rohitmalhotra1420 July 30, 2024 07:56

rohitmalhotra1420 requested a review from mishramonalisha76 July 30, 2024 09:35

rohitmalhotra1420 assigned kalashshah Jul 30, 2024

mishramonalisha76 suggested changes Jul 30, 2024

View reviewed changes

chore: update alert ds acc to review

35ba1fe

kalashshah requested a review from mishramonalisha76 July 31, 2024 05:29

chore: update component acc to review

91c1e0f

rohitmalhotra1420 approved these changes Jul 31, 2024

View reviewed changes

rohitmalhotra1420 merged commit 9d62bc6 into main Jul 31, 2024
2 checks passed

corlard3y pushed a commit that referenced this pull request Jul 31, 2024

feat: add alert ds component (#1781)

6f36a02

* feat: add alert ds component * chore: update alert ds acc to review * chore: update component acc to review

rohitmalhotra1420 linked an issue Aug 1, 2024 that may be closed by this pull request

🚀 [New Feature] - Create an Alert for displaying error, success, warning texts #1759

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add alert ds component #1781

feat: add alert ds component #1781

kalashshah commented Jul 30, 2024

github-actions bot commented Jul 30, 2024

github-actions bot commented Jul 30, 2024 •

edited

Loading

rohitmalhotra1420 commented Jul 30, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

kalashshah Jul 31, 2024

rohitmalhotra1420 Jul 31, 2024

kalashshah Jul 31, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

kalashshah Jul 31, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

kalashshah Jul 31, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

mishramonalisha76 Jul 30, 2024

github-actions bot commented Jul 31, 2024

github-actions bot commented Jul 31, 2024

	/* Child react nodes rendered by Box */
	/* Child react nodes rendered by Alert */

feat: add alert ds component #1781

feat: add alert ds component #1781

Conversation

kalashshah commented Jul 30, 2024

Pull Request Template

Ticket Number

Description

Type of Change

Checklist

Frontend Guidelines

Build & Testing

Screenshots/Video with Explanation

Review & Approvals

Notes

github-actions bot commented Jul 30, 2024

github-actions bot commented Jul 30, 2024 • edited Loading

rohitmalhotra1420 commented Jul 30, 2024

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

github-actions bot commented Jul 31, 2024

github-actions bot commented Jul 31, 2024

github-actions bot commented Jul 30, 2024 •

edited

Loading