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

Added features to the Settings Page and a test file again #61

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

DillonDavidson
Copy link
Contributor

@DillonDavidson DillonDavidson commented Mar 27, 2024

Overview

My bad team, I MAJORLY messed up the last one, so I just remade the pull request.
Created some features for the Settings Page (changing text size across pages, dark mode, a logout button skeleton, a delete account button skeleton, and a retake quiz button skeleton). The skeleton buttons are currently placeholders and will be fililed out as the project develops. More features will be added as the project progresses. Also, I cleaned up the pages directory.

Related Issue(s)

Made the Settings Page do stuff, but no posted Issue.

Changes

Created global font sizes for titles, text, and buttons to be modified by the Text Size buttons on the Settings page. Also, a "darkMode" class has been added to the .css files in the pages directory to facilitate the dark mode theme. Cleaned up the pages directory with specific directories for each page, and modified includes to appropriately handle that. I also created a test file for the Settings Page.

Added

  • app/src/tests/SettingsPage.test.js
  • app/src/contexts/FontSizeContext.js
  • app/src/pages/settings/SettingsPage.js
  • app/src/pages/settings/SettingsPage.module.css
  • app/src/pages/skeleton/Skeleton.js
  • app/src/pages/skeleton/Skeleton.module.css
  • app/src/pages/quests/QuestsPage.js
  • app/src/pages/login/Login.css
  • app/src/pages/login/Login.js

Changed

  • app/src/App.js
  • app/src/global.css
  • app/src/pages/home/Home.module.css

Removed

  • app/src/pages/SettingsPage.js
  • app/src/pages/SettingsPage.module.css
  • app/src/pages/Skeleton.js
  • app/src/pages/Skeleton.module.css
  • app/src/pages/Login.css
  • app/src/pages/Login.js

Fixed

  • N/A

Screenshots

image

Instructions for Testing

npm install
npm start

Checklist before Merging

  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate).
  • I have ensured the changes adhere to the project's coding standards and guidelines.

Additional Comments

I will make the Settings Page look not awful, along with flushing out the skeleton buttons once the pieces for the project needed to link it up to are there. Also, I will work on making increasing our testing coverage.

Copy link
Contributor

@Alexmc567 Alexmc567 left a comment

Choose a reason for hiding this comment

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

Readability:

Code is clear and understandable.

Code Quality:

Code accurately does what it is supposed to do. Lack of comments on some parts of the added code however.

Testing:

Does not fully hit the 90%, but close enough for now. Getting features up is more important than the missing ~15% coverage.

Functionality:

Features appear to work as intended.
Merge conflicts are the only issue that need to be resolved.

Copy link
Contributor

@dusek2 dusek2 left a comment

Choose a reason for hiding this comment

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

Readability:

Code is clear and understandable.

Code Quality:

Code seems reasonably formatted and written

Testing:

Need more coverage

Functionality:

Good addition to settings functionality

@DillonDavidson DillonDavidson merged commit 6aa1f11 into UNLV-CS472-672:main Mar 27, 2024
1 check passed
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.

3 participants