Fixed: Added a button to close model. #1312

ZendeAditya · 2025-01-11T05:50:35Z

Date: 13/01/2025

Developer Name: Aditya Zende (aditya-zende-1)

Issue Ticket Number

Closes [FIX]: modal on the status site to update the task #1313

Description

In the reusable main Model component, I added a close button with an onclick event handler and code to close the model with an esc button.

loom-video.6.mp4

Documentation Updated?

Yes
No

Under Feature Flag

Yes
No

Database Changes

Yes
No

Breaking Changes

Yes
No

Development Tested?

Yes
No

Screenshots

Screenshot 1

Test Coverage

Screenshot 1

Additional Notes

vercel · 2025-01-11T05:50:39Z

@ZendeAditya is attempting to deploy a commit to the RDS-Team Team on Vercel.

A member of the Team first needs to authorize it.

shobhan-sundar-goutam · 2025-01-11T06:24:18Z

Why package.json is committed?
This part is not correct. Please check in other PRs on how to do it correctly.
Do you already have any design for the close button? If not, I would suggest you to check other modals of RDS for the close button design.

AnujChhikara

Build is failing, likely due to the Node.js version upgrade. Please check

tejaskh3 · 2025-01-11T07:59:17Z

The build is failing, please fix that.

tejaskh3 · 2025-01-11T07:59:41Z

Date: 11/01/2025

Developer Name: Aditya Zende (aditya-zende-1)

Issue Ticket Number

#1313

Description

In the reusable main Model component, I added a close button with an onclick event handler and a code to close the model with an esc button.

loom-video.4.mp4

Documentation Updated?

Yes

[ ✅] No

Under Feature Flag

Yes

[ ✅] No

Database Changes

Yes

[ ✅] No

Breaking Changes

Yes

[ ✅] No

Development Tested?

Yes

[ ✅] No

Screenshots

Screenshot 1

Test Coverage

Screenshot 1

Additional Notes

Why is development tested no here?

tejaskh3 · 2025-01-11T08:01:18Z

package.json

-  },
-  "repository": {
-    "type": "git",
-    "url": "https://github.com/Real-Dev-Squad/website-status.git"
-  },
-  "keywords": [],
-  "author": "",
-  "license": "ISC",
-  "dependencies": {
-    "@reduxjs/toolkit": "^1.9.1",
-    "@testing-library/user-event": "^14.5.2",
-    "axios": "^1.2.2",
-    "framer-motion": "^11.0.6",
-    "markdown-to-jsx": "^7.2.0",
-    "moment": "^2.29.4",
-    "next": "^13.1.1",
-    "next-redux-wrapper": "^8.1.0",
-    "react": "^18.2.0",
-    "react-beautiful-dnd": "^13.1.0",
-    "react-calendar": "^4.8.0",
-    "react-dom": "^18.2.0",
-    "react-icons": "^4.12.0",
-    "react-redux": "^8.0.5",
-    "react-toastify": "^9.0.8",
-    "sass": "^1.45.2",
-    "sass-loader": "^13.0.2"
-  },
-  "devDependencies": {
-    "@babel/core": "^7.16.7",
-    "@storybook/addon-actions": "^6.4.9",
-    "@storybook/addon-essentials": "^6.4.9",
-    "@storybook/addon-links": "^6.4.9",
-    "@storybook/builder-webpack5": "^6.4.9",
-    "@storybook/manager-webpack5": "^6.4.9",
-    "@storybook/preset-scss": "^1.0.3",
-    "@storybook/react": "^6.4.9",
-    "@testing-library/dom": "^9.0.1",
-    "@testing-library/jest-dom": "^5.16.1",
-    "@testing-library/react": "^13.3.0",
-    "@testing-library/react-hooks": "^8.0.1",
-    "@types/jest": "^29.2.5",
-    "@types/next": "^9.0.0",
-    "@types/node": "^18.7.8",
-    "@types/react": "^18.0.17",
-    "@types/react-beautiful-dnd": "^13.1.2",
-    "@types/react-dom": "^18.0.6",
-    "@typescript-eslint/eslint-plugin": "^5.57.0",
-    "@typescript-eslint/parser": "^5.57.0",
-    "babel-jest": "^29.3.1",
-    "babel-loader": "^9.1.2",
-    "css-loader": "^6.5.1",
-    "eslint": "^8.6.0",
-    "eslint-config-next": "^13.1.1",
-    "eslint-plugin-react": "^7.32.2",
-    "husky": "8.0.3",
-    "identity-obj-proxy": "^3.0.0",
-    "jest": "^29.3.1",
-    "jest-environment-jsdom": "^29.3.1",
-    "jest-mock-axios": "^4.5.0",
-    "lint-staged": "^13.1.0",
-    "local-ssl-proxy": "^1.3.0",
-    "msw": "^0.49.2",
-    "npm-run-all": "^4.1.5",
-    "pinst": "^3.0.0",
-    "react-hooks-testing-library": "^0.6.0",
-    "react-test-renderer": "^18.2.0",
-    "style-loader": "^3.3.1",
-    "ts-jest": "^29.0.3",
-    "ts-node": "^10.4.0",
-    "tslib": "^2.3.1",
-    "typescript": "^4.5.4",
-    "webpack": "^5.76.0",
-    "whatwg-fetch": "^3.6.2"
-  },
-  "engines": {
-    "node": "18.x"
-  },
-  "lint-staged": {
-    "*.+(ts|tsx)": [
-      "eslint ."
-    ]
-  },
-  "msw": {
-    "workerDirectory": "public"
-  },
-  "volta": {
-    "node": "18.14.2",
-    "yarn": "1.22.19"
-  }
+    "name": "status",
+    "version": "1.0.0",
+    "description": "Shows a status of the ongoing projects being done",
+    "main": "index.js",
+    "scripts": {
+        "run-command:dev-server": "next dev -p 3000",
+        "run-command:reverse-ssl": "local-ssl-proxy --source 443 --target 3000",
+        "dev": "run-p run-command:*",
+        "build:staging": "cp .env.development .env.local && next build",
+        "build": "next build",
+        "start": "next start -p $PORT",


Why is this changed?

please do run prettier and lint fix commands

tejaskh3 · 2025-01-11T08:02:56Z

src/components/Modal/index.tsx

+                        <button
+                            className={styles.closeButton}
+                            onClick={props.toggle}
+                        >
+                            close
+                        </button>
                        {props.children}


can we have a cross icon instead a close button?

If not could you please verify if we are performing close action using the same way at all other modals?

Yes because I added this code in the reusable Model component.

tejaskh3 · 2025-01-11T08:03:36Z

src/components/Modal/modal.module.scss

+    padding: 4px 25px;
+    border-radius: 3px;
+    &:hover {
+        background: #041187;


If possible please try to use global css variables

ZendeAditya · 2025-01-11T08:37:00Z

Hey @tejaskh3 @AnujChhikara Please check it now i fix the node version back to 18

AnujChhikara · 2025-01-11T08:38:59Z

Hey @tejaskh3 @AnujChhikara Please check it now i fix the node version back to 18

where? can you please check if you have pushed latest changes.

ZendeAditya · 2025-01-11T08:45:32Z

Hey @tejaskh3 @AnujChhikara Please check it now i fix the node version back to 18

where? can you please check if you have pushed latest changes.

I restored it back

tejaskh3 · 2025-01-12T18:55:09Z

as the implementation is changed, can you please update the video or screenshot?

ZendeAditya · 2025-01-13T07:20:50Z

as the implementation is changed, can you please update the video or screenshot?

Yes I will update it

ZendeAditya · 2025-01-13T11:09:37Z

@tejaskh3 updated image and video with a new one

pankajjs

Left comments

pankajjs · 2025-01-14T14:27:49Z

src/components/Modal/index.tsx

+                        <button
+                            className={styles.closeButton}
+                            onClick={props.toggle}
+                        >
+                            <RxCross2 size={20} />
+                        </button>


Why this fix is not behind feature flag?

I will update it as a feature flag

pankajjs · 2025-01-14T14:28:29Z

src/components/Modal/index.tsx

+                            className={styles.closeButton}
+                            onClick={props.toggle}
+                        >
+                            <RxCross2 size={20} />


Are we using the button from RDS design system?

ZendeAditya · 2025-01-15T06:53:17Z

Can you please check this @tejaskh3 @pankajjs

AnujChhikara · 2025-01-16T16:32:22Z

Hello @ZendeAditya can you please write test for you changes and attach test coverage in the description

ZendeAditya · 2025-01-16T16:46:21Z

Hello @ZendeAditya can you please write test for you changes and attach test coverage in the description

Yes

tejaskh3

Instead the icon you used,

Please use the icon that is shown in the screenshot, if it is not their please get that from dashboard site.

ZendeAditya · 2025-01-17T00:54:04Z

Instead the icon you used,

Please use the icon that is shown in the screenshot, if it is not their please get that from dashboard site.

Ok I will add it

ZendeAditya · 2025-01-17T15:08:25Z

@tejaskh3 @AnujChhikara
I have added the icon according to the screenshot and also wrote a test case for my code. please review again.

ZendeAditya · 2025-01-18T15:28:19Z

@tejaskh3 @AnujChhikara I have added the icon according to the screenshot and also wrote a test case for my code. please review again.

please check this

pankajjs · 2025-01-19T11:10:50Z

Please make your changes behind feature flag.
Thank you.

ZendeAditya · 2025-01-19T15:05:00Z

Please make your changes behind feature flag. Thank you.

Means?

AnujChhikara requested changes Jan 11, 2025

View reviewed changes

tejaskh3 requested changes Jan 11, 2025

View reviewed changes

chore: remove unnecessary changes

6cb306c

ZendeAditya force-pushed the fix/model-form branch from 60b7ec3 to 6cb306c Compare January 12, 2025 13:02

Fix button css

71e6959

ZendeAditya requested review from tejaskh3 and AnujChhikara January 12, 2025 15:12

Achintya-Chatterjee assigned ZendeAditya Jan 13, 2025

pankajjs requested changes Jan 14, 2025

View reviewed changes

ZendeAditya requested a review from pankajjs January 15, 2025 13:29

tejaskh3 requested changes Jan 16, 2025

View reviewed changes

ZendeAditya added 2 commits January 17, 2025 20:31

Added simple button and Write testcase for button

7ad50d5

Added simple button and Write testcase for button

c57ec97

ZendeAditya requested a review from tejaskh3 January 17, 2025 15:14

Fixed: Added a button to close model. #1312

Are you sure you want to change the base?

Fixed: Added a button to close model. #1312

Conversation

ZendeAditya commented Jan 11, 2025 • edited Loading

Issue Ticket Number

Description

Documentation Updated?

Under Feature Flag

Database Changes

Breaking Changes

Development Tested?

Screenshots

Test Coverage

Additional Notes

vercel bot commented Jan 11, 2025

shobhan-sundar-goutam commented Jan 11, 2025

AnujChhikara left a comment • edited Loading

Choose a reason for hiding this comment

tejaskh3 commented Jan 11, 2025

tejaskh3 commented Jan 11, 2025

Issue Ticket Number

Description

Documentation Updated?

Under Feature Flag

Database Changes

Breaking Changes

Development Tested?

Screenshots

Test Coverage

Additional Notes

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

ZendeAditya commented Jan 11, 2025

AnujChhikara commented Jan 11, 2025

ZendeAditya commented Jan 11, 2025

tejaskh3 commented Jan 12, 2025

ZendeAditya commented Jan 13, 2025

ZendeAditya commented Jan 13, 2025 • edited Loading

pankajjs 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

ZendeAditya commented Jan 15, 2025 • edited Loading

AnujChhikara commented Jan 16, 2025

ZendeAditya commented Jan 16, 2025

tejaskh3 left a comment

Choose a reason for hiding this comment

ZendeAditya commented Jan 17, 2025

ZendeAditya commented Jan 17, 2025

ZendeAditya commented Jan 18, 2025

pankajjs commented Jan 19, 2025

ZendeAditya commented Jan 19, 2025

ZendeAditya commented Jan 11, 2025 •

edited

Loading

AnujChhikara left a comment •

edited

Loading

ZendeAditya commented Jan 13, 2025 •

edited

Loading

ZendeAditya commented Jan 15, 2025 •

edited

Loading