feat: create popup base layout for forms #1

ahtesham-quraish · 2024-03-26T07:43:53Z

Description

Created basic layout using paragon modal for forms
Created privacy policy footer for signup form that would be controlled by flag

JIRA

How Has This Been Tested?

It has been tested locally.

Screenshots:

After

src/base-container/index.jsx

attiyaIshaque · 2024-03-26T07:52:26Z

src/base-container/index.jsx

+      </div>
+    </ModalDialog.Body>
+    <ModalDialog.Footer className="modal-footer-content">
+      {isPrivacyPolicy && (<PrivacyPolicy />)}


can we do this like {isPrivacyPolicy && <PrivacyPolicy />}?

it won't be any different

syedsajjadkazmii

We can discuss this as well but I was expecting BaseContainer to be inside the ModalDialog body since content is inside the modal.

I was expecting a hierary such as: we have an authn component, lets say AuthnComponent which renders ModalDialog. Inside that ModalDialog, we have BaseContainer. Inside BaseContainer we pass the content (Register or login form). Then we use the AuthnComponent to render as final component.

And then AuthnComponent should be the one to receive open and onClose props.

example/index.jsx

src/base-container/index.scss

Description: Create popup basic layout for forms VAN-1840

src/authn-component/index.jsx

zainab-amir

The background color for the modal is light-400
We should discuss how we will use the registration & login components in paragon and which component should accept the the form

zainab-amir · 2024-03-26T10:00:11Z

example/index.jsx

Lets not update this file for now. I will create a separate ticket to handle how we can view different forms.

Please remove changes from this file.

src/base-container/PrivacyPolicyFooter.jsx

src/base-container/layout/LargeLayout.jsx

src/base-container/index.scss

src/authn-component/PrivacyPolicyFooter.jsx

src/base-container/index.jsx

zainab-amir · 2024-03-26T10:40:12Z

src/base-container/layout/LargeLayout.jsx

We will most likely not have promo content for small screen and this component feels like an overkill. We can easily incorporate this in the Modal content directly.
See my comment about abstraction.

ok my assumption was that we would have small layout for promo as well that is why I did that abstraction

src/authn-component/index.scss

syedsajjadkazmii

Could you please add picture of whole screen along with current ss of just modal?

src/authn-component/index.jsx

src/authn-component/index.scss

src/authn-component/index.jsx

src/authn-component/index.scss

syedsajjadkazmii · 2024-03-27T10:18:23Z

NIT: Since we are writing new components, can we adopt the practice of putting docstring to each Component and function? Doesn't matter if it's just a line description.

Doing this while writing components wouldn't be much effort IMO.

zainab-amir · 2024-03-28T03:58:21Z

example/index.jsx

Please remove changes from this file.

zainab-amir · 2024-03-28T04:02:00Z

src/authn-component/index.jsx

+/**
+ * A Authn component that would render the register or login component with condtional logic.
+ *
+ * @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.


Suggested change

* @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.

* @param {boolean} open - Required. Whether to open the modal window containing login or registration form

zainab-amir · 2024-03-28T04:02:18Z

src/authn-component/index.jsx

+ * @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.
+ * @param {function} setOpen - Required. Is used to toggle the modal window's open flag.
+ *
+ * @returns {JSX.Element} The rendered Login or register component.


Suggested change

* @returns {JSX.Element} The rendered Login or register component.

* @returns {JSX.Element} The rendered BaseContainer component containing either login or registration form.

zainab-amir · 2024-03-28T04:03:02Z

src/base-container/index.jsx

+import './index.scss';
+
+/**
+ * A Base component that would render the paragon modal and content of registration or login.


Suggested change

* A Base component that would render the paragon modal and content of registration or login.

* Base component for registration or login form modals.

zainab-amir · 2024-03-28T04:03:36Z

src/base-container/index.jsx

+/**
+ * A Base component that would render the paragon modal and content of registration or login.
+ *
+ * @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.


Suggested change

* @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.

* @param {boolean} open - Required. Whether to open the modal window

zainab-amir · 2024-03-28T04:05:13Z

src/base-container/index.jsx

+ * @param {string} footerText - Optional. This is used to render the footer text.
+ * @param {React.node} children - Required. It contains the React node of registration page or login page.


Suggested change

* @param {string} footerText - Optional. This is used to render the footer text.

* @param {React.node} children - Required. It contains the React node of registration page or login page.

* @param {string} footerText - The text for the modal footer

* @param {React.node} children - Required. The login or registration form.

zainab-amir · 2024-03-28T04:06:00Z

src/base-container/index.jsx

+
+);
+
+BaseContainer.defaultProps = {


[This is just to make the components consistent] Please move default props below the actual props.

zainab-amir · 2024-03-28T04:06:41Z

src/base-container/index.jsx

+ */
+
+const BaseContainer = ({
+  children, open, setOpen, footerText,


[This is just to make the components consistent] Please sort these.

zainab-amir · 2024-03-28T04:10:13Z

src/base-container/index.jsx

+ * @param {string} footerText - Optional. This is used to render the footer text.
+ * @param {React.node} children - Required. It contains the React node of registration page or login page.
+ *
+ * @returns {JSX.Element} The rendered BaseContainer component.


Suggested change

* @returns {JSX.Element} The rendered BaseContainer component.

* @returns {JSX.Element} The rendered login or registration form modal.

zainab-amir · 2024-03-28T04:11:47Z

src/authn-component/index.jsx

+import BaseContainer from '../base-container';
+
+/**
+ * A Authn component that would render the register or login component with condtional logic.


Suggested change

* A Authn component that would render the register or login component with condtional logic.

* Main component that holds the logic for conditionally rendering login or registration form.

syedsajjadkazmii

Please update screenshot of modal. (In full screen)

src/base-container/index.jsx

zainab-amir · 2024-03-28T05:51:52Z

src/base-container/index.jsx

+      <ModalDialog.Footer className="bg-dark-500 p-0">
+        <p className="p-4.5 mb-0 text-white">{footerText}</p>
+      </ModalDialog.Footer>


Suggested change

<ModalDialog.Footer className="bg-dark-500 p-0">

<p className="p-4.5 mb-0 text-white">{footerText}</p>

</ModalDialog.Footer>

<ModalDialog.Footer className="bg-dark-500 p-4.5">

<p className="mb-0 text-white">{footerText}</p>

</ModalDialog.Footer>

It works fine either way btw but anyway I have moved this class to footer

zainab-amir

Please address the comment before merging

zainab-amir · 2024-03-28T06:40:30Z

src/base-container/index.jsx

+BaseContainer.defaultProps = {
+  footerText: 'By creating an account, you agree to the Terms of Service and Honor Code and you acknowledge that edX and each Member process your personal data in accordance with the Privacy Policy.',
+};


Is this some testing code you missed?

Suggested change

BaseContainer.defaultProps = {

footerText: 'By creating an account, you agree to the Terms of Service and Honor Code and you acknowledge that edX and each Member process your personal data in accordance with the Privacy Policy.',

};

BaseContainer.defaultProps = {

footerText: '',

};

Description: Refactor the structure of base component VAN-1840

attiyaIshaque reviewed Mar 26, 2024

View reviewed changes

src/base-container/index.jsx Show resolved Hide resolved

attiyaIshaque reviewed Mar 26, 2024

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/van-1840 branch 2 times, most recently from 95040a4 to bb70805 Compare March 26, 2024 08:07

syedsajjadkazmii reviewed Mar 26, 2024

View reviewed changes

example/index.jsx Outdated Show resolved Hide resolved

src/base-container/index.scss Outdated Show resolved Hide resolved

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from bb70805 to 95eb761 Compare March 26, 2024 09:21

feat: create popup base layout for forms

3953c76

Description: Create popup basic layout for forms VAN-1840

ahtesham-quraish force-pushed the ahtesham/van-1840 branch 2 times, most recently from 8bf2dda to 4c9485c Compare March 26, 2024 10:25

syedsajjadkazmii reviewed Mar 26, 2024

View reviewed changes

src/authn-component/index.jsx Outdated Show resolved Hide resolved

zainab-amir suggested changes Mar 26, 2024

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/van-1840 branch 3 times, most recently from b4473ba to faea39f Compare March 27, 2024 07:35

syedsajjadkazmii reviewed Mar 27, 2024

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from faea39f to 47d1d15 Compare March 27, 2024 09:18

syedsajjadkazmii reviewed Mar 27, 2024

View reviewed changes

src/authn-component/index.scss Outdated Show resolved Hide resolved

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from 47d1d15 to 52c525e Compare March 27, 2024 09:50

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from 52c525e to 10ebf62 Compare March 27, 2024 11:57

zainab-amir reviewed Mar 28, 2024

View reviewed changes

syedsajjadkazmii reviewed Mar 28, 2024

View reviewed changes

src/base-container/index.jsx Outdated Show resolved Hide resolved

zainab-amir reviewed Mar 28, 2024

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from 10ebf62 to 9d4f74b Compare March 28, 2024 06:15

syedsajjadkazmii approved these changes Mar 28, 2024

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/van-1840 branch 2 times, most recently from d85ad98 to eb6d9ef Compare March 28, 2024 06:39

zainab-amir approved these changes Mar 28, 2024

View reviewed changes

refactor: refactor the structure of base container

832194d

Description: Refactor the structure of base component VAN-1840

ahtesham-quraish force-pushed the ahtesham/van-1840 branch from eb6d9ef to 832194d Compare March 28, 2024 06:42

ahtesham-quraish merged commit e941b18 into master Mar 28, 2024
5 checks passed

ahtesham-quraish deleted the ahtesham/van-1840 branch March 28, 2024 06:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create popup base layout for forms #1

feat: create popup base layout for forms #1

ahtesham-quraish commented Mar 26, 2024 •

edited

Loading

attiyaIshaque Mar 26, 2024

ahtesham-quraish Mar 26, 2024

syedsajjadkazmii left a comment •

edited

Loading

zainab-amir left a comment •

edited

Loading

zainab-amir Mar 26, 2024

zainab-amir Mar 28, 2024

zainab-amir Mar 26, 2024

ahtesham-quraish Mar 27, 2024 •

edited

Loading

syedsajjadkazmii left a comment

syedsajjadkazmii commented Mar 27, 2024

zainab-amir Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir Mar 28, 2024

syedsajjadkazmii left a comment

zainab-amir Mar 28, 2024

ahtesham-quraish Mar 28, 2024

zainab-amir left a comment

zainab-amir Mar 28, 2024

	* @param {boolean} open - Required. Open veriable that will be used to toggle the modal window.
	* @param {boolean} open - Required. Whether to open the modal window containing login or registration form

	* @returns {JSX.Element} The rendered Login or register component.
	* @returns {JSX.Element} The rendered BaseContainer component containing either login or registration form.

	* A Base component that would render the paragon modal and content of registration or login.
	* Base component for registration or login form modals.

		* @param {string} footerText - Optional. This is used to render the footer text.
		* @param {React.node} children - Required. It contains the React node of registration page or login page.

	* A Authn component that would render the register or login component with condtional logic.
	* Main component that holds the logic for conditionally rendering login or registration form.

feat: create popup base layout for forms #1

feat: create popup base layout for forms #1

Conversation

ahtesham-quraish commented Mar 26, 2024 • edited Loading

Description

JIRA

How Has This Been Tested?

Screenshots:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syedsajjadkazmii left a comment • edited Loading

Choose a reason for hiding this comment

zainab-amir left a comment • 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

ahtesham-quraish Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

syedsajjadkazmii left a comment

Choose a reason for hiding this comment

syedsajjadkazmii commented Mar 27, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syedsajjadkazmii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zainab-amir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahtesham-quraish commented Mar 26, 2024 •

edited

Loading

syedsajjadkazmii left a comment •

edited

Loading

zainab-amir left a comment •

edited

Loading

ahtesham-quraish Mar 27, 2024 •

edited

Loading