Skip to content

Commit

Permalink
changes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
in-mai-space committed Mar 23, 2024
1 parent 5ed4541 commit 0798640
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 18,413 deletions.
29 changes: 14 additions & 15 deletions frontend/sac-mobile/app/(auth)/_components/registration-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,19 @@ type RegisterFormData = {
passwordConfirm: string;
};

const registerSchema = z
.object({
firstName: z.string().min(2, {
message: 'First name must be at least 2 characters long'
}),
lastName: z.string().min(2, {
message: 'Last name must be at least 2 characters long'
}),
email: z.string().email({ message: 'Invalid email' }),
password: z
.string()
.min(8, { message: 'Password must be at least 8 characters long' }),
passwordConfirm: z.string()
})
const registerSchema = z.object({
firstName: z.string().min(2, {
message: 'First name must be at least 2 characters long'
}),
lastName: z.string().min(2, {
message: 'Last name must be at least 2 characters long'
}),
email: z.string().email({ message: 'Invalid email' }),
password: z
.string()
.min(8, { message: 'Password must be at least 8 characters long' }),
passwordConfirm: z.string()
});

const RegistrationForm = () => {
const {
Expand Down Expand Up @@ -132,7 +131,7 @@ const RegistrationForm = () => {
<Input
title="Email"
autoCorrect={false}
placeholder="Northeastern email"
placeholder="[email protected]"
onChangeText={onChange}
value={value}
onSubmitEditing={handleSubmit(onSubmit)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState } from 'react';
import { useForm } from 'react-hook-form';
import { Alert, SafeAreaView, ScrollView, Text, View } from 'react-native';
import { Alert, ScrollView, Text, View } from 'react-native';

import { router } from 'expo-router';

Expand All @@ -9,21 +9,17 @@ import { ZodError } from 'zod';
import { Button } from '@/components/button';
import Error from '@/components/error';
import { categories } from '@/lib/const';
import { allTags } from '@/lib/utils';
import { Category } from '@/types/categories';

type TagsData = {
type UserInterestsData = {
tags: String[];
};

// combine all tags of different categories into one for All tab
let allTags: string[] = [];
for (let i = 0; i < categories.length; i++) {
allTags = allTags.concat(categories[i].tags);
}
const categoriesMenu = [{ name: 'All', tags: allTags }, ...categories];
const categoriesMenu = [{ name: 'All', tags: allTags() }, ...categories];

const TagForm = () => {
const { handleSubmit } = useForm<TagsData>();
const UserInterestsForm = () => {
const { handleSubmit } = useForm<UserInterestsData>();
const [selectedTags, setSelectedTags] = useState<String[]>([]);
const [buttonClicked, setButtonClicked] = useState<boolean>(false);
const [selectedCategory, setSelectedCategory] = useState('All');
Expand All @@ -44,7 +40,7 @@ const TagForm = () => {
}
};

const onSubmit = (data: TagsData) => {
const onSubmit = (data: UserInterestsData) => {
setButtonClicked(true);
if (selectedTags.length === 0) {
return;
Expand All @@ -65,7 +61,7 @@ const TagForm = () => {
const heightAdjust =
selectedTags.length === 0 && buttonClicked ? 'h-[46%]' : 'h-[47%]';

const Tag = (props: { tag: string;}) => {
const Tag = (props: { tag: string }) => {

Check warning on line 64 in frontend/sac-mobile/app/(auth)/_components/user-interest-form.tsx

View workflow job for this annotation

GitHub Actions / Lint

Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state (https://reactjs.org/docs/reconciliation.html#elements-of-different-types). Instead, move this component definition out of the parent component “UserInterestsForm” and pass data as props
return (
<Button
onPress={() => handleTagPress(props.tag)}
Expand Down Expand Up @@ -104,7 +100,9 @@ const TagForm = () => {
<ScrollView className={heightAdjust}>
<View className="flex-row flex-wrap">
{selectedCategory === 'All'
? allTags.map((tag, key) => <Tag tag={tag} key={key} />)
? allTags().map((tag, key) => (
<Tag tag={tag} key={key} />
))
: categories
.find((c) => c.name === selectedCategory)
?.tags.map((tag, key) => (
Expand All @@ -126,4 +124,4 @@ const TagForm = () => {
);
};

export default TagForm;
export default UserInterestsForm;
4 changes: 2 additions & 2 deletions frontend/sac-mobile/app/(auth)/_layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ const AuthLayout = () => {
}}
/>
<Stack.Screen
name="tags"
name="user-interests"
options={{
title: 'Tags',
title: 'User Interests',
headerShown: false
}}
/>
Expand Down
2 changes: 1 addition & 1 deletion frontend/sac-mobile/app/(auth)/login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const Login = () => {
<View className="flex-1">
<View className="px-[8%] pb-[10%]">
<View className="pt-[1%]">
<Wordmark textColor="text-white" />
<Wordmark variant="secondary" />
</View>
<View className="pt-[9.5%] pb-[6%]">
<Text className="text-5xl font-bold text-white">
Expand Down
2 changes: 1 addition & 1 deletion frontend/sac-mobile/app/(auth)/register.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const Register = () => {
<View className="px-[8%] pb-[9%]">
<View className="flex flex-row justify-between mx-auto w-full items-center pt-[3%] pb-[5.5%]">
<View className="pl-[0.5%]">
<Wordmark textColor="text-white" />
<Wordmark variant="secondary" />
</View>
<Button
onPress={() => router.push('/(auth)/login')}
Expand Down
33 changes: 0 additions & 33 deletions frontend/sac-mobile/app/(auth)/tags.tsx

This file was deleted.

10 changes: 3 additions & 7 deletions frontend/sac-mobile/app/(auth)/user-details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,10 @@ const UserDetails = () => {
const UserDetailsSchema = z.object({
college: z.string(),
major: z.string().array(),
graduationYear: z.string(),
graduationYear: z.string()
});

const onSubmit = ({
major,
college,
graduationYear
}: UserDetailsForm) => {
const onSubmit = ({ major, college, graduationYear }: UserDetailsForm) => {

Check warning on line 39 in frontend/sac-mobile/app/(auth)/user-details.tsx

View workflow job for this annotation

GitHub Actions / Lint

'major' is already declared in the upper scope on line 16 column 10

Check warning on line 39 in frontend/sac-mobile/app/(auth)/user-details.tsx

View workflow job for this annotation

GitHub Actions / Lint

'college' is already declared in the upper scope on line 15 column 10

Check warning on line 39 in frontend/sac-mobile/app/(auth)/user-details.tsx

View workflow job for this annotation

GitHub Actions / Lint

'graduationYear' is already declared in the upper scope on line 17 column 10
try {
const updatedData = {
major,
Expand All @@ -49,7 +45,7 @@ const UserDetails = () => {
};
UserDetailsSchema.parse(updatedData);
Alert.alert('Form Submitted', JSON.stringify(updatedData));
router.push('/(auth)/tags');
router.push('/(auth)/user-interests');
} catch (error) {
if (error instanceof ZodError) {
Alert.alert('Validation Error', error.errors[0].message);
Expand Down
24 changes: 24 additions & 0 deletions frontend/sac-mobile/app/(auth)/user-interests.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import { SafeAreaView, Text, View } from 'react-native';

import Wordmark from '@/components/wordmark';

import UserInterestsForm from './_components/user-interest-form';

const UserInterests = () => {
return (
<SafeAreaView>
<View className="px-[8%] pt-[4%]">
<View className="flex flex-row">
<Wordmark />
</View>
<Text className="text-5xl pt-[6%] pb-[5%] font-bold">
What are you interested in?
</Text>
<UserInterestsForm />
</View>
</SafeAreaView>
);
};

export default UserInterests;
6 changes: 4 additions & 2 deletions frontend/sac-mobile/app/(auth)/verification.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Button } from '@/components/button';
import { router } from 'expo-router';
import React from 'react';
import { ScrollView, Text, View } from 'react-native';

Check failure on line 2 in frontend/sac-mobile/app/(auth)/verification.tsx

View workflow job for this annotation

GitHub Actions / Lint

'Text' is defined but never used

Check failure on line 2 in frontend/sac-mobile/app/(auth)/verification.tsx

View workflow job for this annotation

GitHub Actions / Lint

'View' is defined but never used

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import ScrollView.
import { SafeAreaView } from 'react-native-safe-area-context';

import { router } from 'expo-router';

import { Button } from '@/components/button';

const Verification = () => {
return (
<SafeAreaView className="h-full bg-neutral-500" edges={['top']}>
Expand Down
18 changes: 9 additions & 9 deletions frontend/sac-mobile/components/spinner.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
import React from 'react';
import { View, ActivityIndicator, Text } from 'react-native';
import { ActivityIndicator, Text, View } from 'react-native';

import { VariantProps, cva } from 'class-variance-authority';

import { cn } from '@/lib/utils';

const spinnerVariants = {
size: {
default: 'w-6 h-6',
large: 'w-8 h-8',
small: 'w-4 h-4',
small: 'w-4 h-4'
},
color: {
default: 'text-gray-500',
primary: 'text-blue-500',
secondary: 'text-red-500',
},
secondary: 'text-red-500'
}
};

const spinnerStyles = cva(['items-center', 'justify-center'], {
variants: spinnerVariants,
defaultVariants: {
size: 'default',
color: 'default',
},
color: 'default'
}
});

export interface SpinnerProps
extends VariantProps<typeof spinnerStyles> {
export interface SpinnerProps extends VariantProps<typeof spinnerStyles> {
text?: string;
}

Expand All @@ -41,4 +41,4 @@ const Spinner = ({ size, color, text }: SpinnerProps) => {

Spinner.displayName = 'Spinner';

export { Spinner, spinnerVariants };
export { Spinner, spinnerVariants };
26 changes: 21 additions & 5 deletions frontend/sac-mobile/components/wordmark.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
import { Text, View } from 'react-native';

interface WordmarkProps {
textColor?: string;
}
import { VariantProps, cva } from 'class-variance-authority';

const Wordmark = ({ textColor }: WordmarkProps) => {
import { cn } from '@/lib/utils';

const wordmarkVariants = {
variant: {
default: 'text-black',
secondary: 'text-white'
}
};

const wordmarkStyle = cva(['text-2xl', 'font-bold'], {
variants: wordmarkVariants,
defaultVariants: {
variant: 'default'
}
});

export interface WordmarkProps extends VariantProps<typeof wordmarkStyle> {}

const Wordmark = ({ variant }: WordmarkProps) => {
return (
<View className={`flex flex-row pt-[3%] pb-[5.5%]`}>
<Text className={`text-2xl font-bold ${textColor}`}>Wordmark</Text>
<Text className={cn(wordmarkStyle({ variant }))}>Wordmark</Text>
</View>
);
};
Expand Down
29 changes: 7 additions & 22 deletions frontend/sac-mobile/lib/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const majorArr = [
export const categories = [
{
name: 'Pre-Professional',
tags: ['Pre-med', 'Pre-law', 'Other']
tags: ['Pre-med', 'Pre-law']
},
{
name: 'Cultural and Identity',
Expand All @@ -137,23 +137,16 @@ export const categories = [
'Latin America',
'African American',
'Asian American',
'LGBTQ',
'Other'
'LGBTQ'
]
},
{
name: 'Arts and Creativity',
tags: [
'Performing Arts',
'Visual Arts',
'Creative Writing',
'Music',
'Other'
]
tags: ['Performing Arts', 'Visual Arts', 'Creative Writing', 'Music']
},
{
name: 'Sports and Recreation',
tags: ['Soccer', 'Hiking', 'Climbing', 'Lacrosse', 'Other']
tags: ['Soccer', 'Hiking', 'Climbing', 'Lacrosse']
},
{
name: 'Science and Technology',
Expand All @@ -171,8 +164,7 @@ export const categories = [
'DataScience',
'Mechanical Engineering',
'Electrical Engineering',
'Industrial Engineering',
'Other'
'Industrial Engineering'
]
},
{
Expand All @@ -181,18 +173,11 @@ export const categories = [
'Volunteerism',
'Environmental Advocacy',
'Human Rights',
'Community Outreach',
'Other'
'Community Outreach'
]
},
{
name: 'Media and Communication',
tags: [
'Journalism',
'Broadcasting',
'Film',
'Public Relations',
'Other'
]
tags: ['Journalism', 'Broadcasting', 'Film', 'Public Relations']
}
];
Loading

0 comments on commit 0798640

Please sign in to comment.