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

Settings Page #54

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Settings Page #54

wants to merge 1 commit into from

Conversation

bhavitaap
Copy link
Collaborator

https://linear.app/bread-and-roses/issue/BRE-42/settings-page

created settings cards and hardcoded dummy info

🍞 What's new in this PR

🥐 Description

  • added settings cards for each of the subsections in the settings page
  • hardcoded dummy information (based on the figma)

🥖 Screenshots

🥪 How to review

  • app/settings/page.tsx
  • components/SettingsCard <- 5 new files created in this folder for each of the subsections
  • expected behavior: when navigating to the Settings page, it should look like the Figma
  • feedback: is there possibly a way to collapse the files into one and use props instead of hardcoding the info

🥧 Next steps

  • update the info from the database
  • for bullet point info make it loop through relevant
SettingsPageCards info instead

🥞 Relevant links

🥨 Online sources

🥯 Related PRs

CC: @jxmoose

https://linear.app/bread-and-roses/issue/BRE-42/settings-page

created settings cards and hardcoded dummy info
Copy link
Collaborator

Choose a reason for hiding this comment

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

this shouldn't be added to git but also not sure if this file should exist in first place lol

<P $fontWeight="500" $color={COLORS.gray12} $align="left">
Type of Act
</P>
<ul style={{ paddingLeft: '20px', listStyleType: 'disc' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you change the px to rem! overall should almost always use rem over px. https://nekocalc.com/px-to-rem-converter u can use this website but for smaller ones just divide by 16 and it should be accurate.

<P $fontWeight="500" $color={COLORS.gray12} $align="left">
Genre
</P>
<ul style={{ paddingLeft: '20px', listStyleType: 'disc' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

<P $fontWeight="500" $color={COLORS.gray12} $align="left">
Facility Type
</P>
<ul style={{ paddingLeft: '20px', listStyleType: 'disc' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

<P $fontWeight="500" $color={COLORS.gray12} $align="left">
Location Preferences
</P>
<ul style={{ paddingLeft: '20px', listStyleType: 'disc' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

<P $fontWeight="500" $color={COLORS.gray12} $align="left">
Preferred Audience
</P>
<ul style={{ paddingLeft: '20px', listStyleType: 'disc' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

@@ -44,21 +44,24 @@ export const Content = styled.main`
export const SubHeader = styled.main`
display: flex;
flex-direction: column;
gap: 4px;
gap: 24px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

justify-content: start;
margin-bottom: 0.25rem;
`;

export const SettingDetail = styled.main`
gap: 4px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

^

Copy link
Collaborator

@jxmoose jxmoose left a comment

Choose a reason for hiding this comment

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

overall looks great! just some small changes, if you look at the figma there some of the text should have bullet points if you could add those. also just general things never use px just use rem instead! also delete the .env.local.save file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants