Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[36] – Refactor: NextJs AppDir #54

Merged
merged 16 commits into from
Jul 2, 2024
Merged

[36] – Refactor: NextJs AppDir #54

merged 16 commits into from
Jul 2, 2024

Conversation

Drish-xD
Copy link
Collaborator

@Drish-xD Drish-xD commented Jun 11, 2024

Issues Fixed: #56 #57 #45 #15 #18 #19 #20 #25

Tasks:

  • Convert to NextJs AppDir
  • Add SchadCn UI lib
  • Fix Types used in app
  • Data Table Improvements
  • Form Stepper Improvements
  • Form Builder for easy form creation
  • Server Action Addition

@Drish-xD Drish-xD requested a review from suryabulusu June 23, 2024 12:20
@Drish-xD Drish-xD self-assigned this Jun 23, 2024
@Drish-xD Drish-xD added the enhancement New feature or request label Jun 23, 2024
'id',
'portal_link',
'signup_form_id',
'owner_id',
Copy link
Contributor

Choose a reason for hiding this comment

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

why is owner id part of code..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bcz getting owner_id from api response, so deleting it before creating a new session.

'name',
'id',
'portal_link',
'signup_form_id',
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we can retain this

Copy link
Contributor

Choose a reason for hiding this comment

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

provided signup_form is true

Copy link
Contributor

Choose a reason for hiding this comment

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

but for now its fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay, will change it accordingly when needed

{ label: 'Activate Sign-Up', value: data.is_active ? 'Yes' : 'No' },
{ label: 'Is pop up form allowed', value: data.popup_form ? 'Yes' : 'No' },
{
label: 'Number fields in pop up form',
Copy link
Contributor

Choose a reason for hiding this comment

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

Number of fields in Popup Form

{
label: 'Number fields in pop up form',
value: data.meta_data?.number_of_fields_in_popup_form,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Redirection allowed?

value: data.meta_data?.number_of_fields_in_popup_form,
},
{ label: 'Is redirection allowed', value: data.redirection ? 'Yes' : 'No' },
{ label: 'Is Id generation allowed', value: data.id_generation ? 'Yes' : 'No' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Is ID Generation allowed?

},
{ label: 'Is redirection allowed', value: data.redirection ? 'Yes' : 'No' },
{ label: 'Is Id generation allowed', value: data.id_generation ? 'Yes' : 'No' },
{ label: 'Signup form name', value: data.meta_data?.signup_form_name ?? 'N/A' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Signup Form Name

Copy link
Contributor

Choose a reason for hiding this comment

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

note that this is displayed only if signup_form is true etc .. if signup_form is false then no need to show.

infact if session type is "sign-in" or "sign-in with forgot id" .. signup_form should be false and signup_form_name option shoudln't be displayed at all.

keep in mind for later if not this PR. make an issue in github repo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created a Issue for this #59,

},
{ label: 'Marking Scheme', value: data.meta_data?.marking_scheme },
{ label: 'Optional Limits', value: data.meta_data?.optional_limits },
{ label: 'Show Answers', value: data.meta_data?.show_answers ? 'Yes' : 'No' },
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this not getting deleted in duplicate columns

type: 'select',
options: AuthOptions,
label: 'Auth type',
placeholder: 'Select a auth type',
Copy link
Contributor

Choose a reason for hiding this comment

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

an

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

},
activateSignUp: {
type: 'switch',
label: 'Activate sign up',
Copy link
Contributor

Choose a reason for hiding this comment

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

Activate Sign Up .. just check caps throughout

},
isPopupForm: {
type: 'switch',
label: 'Is pop up form allowed',
Copy link
Contributor

Choose a reason for hiding this comment

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

Popup

},
noOfFieldsInPopup: {
type: 'number',
label: 'No of fields in popup',
Copy link
Contributor

Choose a reason for hiding this comment

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

Number of fields in popup form


return (
<main className="container text-center">
<h2>TO BE IMPLEMENTED {(sessionData as Session)?.name}</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

whats this for

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thought of creating a seperate page for showing the Session details of perticular session.

@@ -0,0 +1,76 @@
'use client';
Copy link
Contributor

Choose a reason for hiding this comment

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

for these ui tsx files.. follow the same naming convention na.. capitalize the file name? Card.tsx, Checkbox.tsx etc..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The thing is using shadCn UI, it creates the Files automatically using cli, and these files have small cases.

auth_type: AuthType.ID,
id_generation: false,
redirection: false,
signup_form: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

why are signup_form_name / signup_form_id not here

Copy link
Contributor

Choose a reason for hiding this comment

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

check for other cases too

infinite_session: false,
date_created: new Date(),
},
purpose: {
Copy link
Contributor

Choose a reason for hiding this comment

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

this will change for liveclass

session_id: '',
meta_data: {
...formData.meta_data,
report_link: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

wont be there for non quiz platform.. next pr

Copy link
Contributor

@suryabulusu suryabulusu left a comment

Choose a reason for hiding this comment

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

left minor comments.. will try to launch on amplify now

@Drish-xD Drish-xD changed the title Refactor: NextJs AppDir [#56] Refactor: NextJs AppDir Jul 1, 2024
Copy link
Contributor

@suryabulusu suryabulusu left a comment

Choose a reason for hiding this comment

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

looks good

@suryabulusu suryabulusu merged commit 16e7440 into main Jul 2, 2024
1 check passed
@Drish-xD Drish-xD changed the title [#56] Refactor: NextJs AppDir [#36] Refactor: NextJs AppDir Aug 5, 2024
@Drish-xD Drish-xD changed the title [#36] Refactor: NextJs AppDir [36] – Refactor: NextJs AppDir Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DMP 2024 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DMP 2024]: Converting Next.js Pages Router to App-Router
2 participants