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

61 front end pages styling #62

Merged
merged 9 commits into from
Mar 4, 2024
Merged

61 front end pages styling #62

merged 9 commits into from
Mar 4, 2024

Conversation

ComeOnConMan
Copy link
Collaborator

@ComeOnConMan ComeOnConMan commented Mar 1, 2024

What does this PR do?

This pull request is simply to merge in the extra front end pages that have been added for the sake of convenience. I have Added a Login, Completion, and very basic skeletal admin page. BONUS: After getting lost in the sauce, all 3 graphical components seen in the Completion page have been created. The components just need a connection to the backend, and work on temporary data as of right now. The word cloud, experience similarity pie chart, and the table with classification information have been added.

Github Issue Number

Resolves: #61
Pages created and ready to begin some more advanced implementation

Relates to:

Author checklist

If any of these points have yet to be satisfied, make sure that you set the title to the format DRAFT #issue-num Title

  • The title is short and descriptive of the PR. Must start with the Github Issue Number (format: #issue-num Title).
  • The description follows proper cl description practices and mentions related Github Issues (make sure this is the first thing you mention).
  • Branch has merged in the latest version of main
  • Linting has occured, as per the project linting config
  • All changed functions have proper docstring/wiki updates (front-end team) to describe what they do and how to use them.
  • Add yourself as the assignee.
  • Add reviewer a reviewer and let them know on discord.
  • Ensure that all relevant ticket has been linked to the PR

Reviewer checklist

  • Relevant issue is mentioned in description
  • Code solves the issue
  • Code follows the specification
  • Code is the best solution for the issue
  • Branch is ahead of main
  • Ensures the fix/feature works locally

@ComeOnConMan ComeOnConMan self-assigned this Mar 1, 2024
@ComeOnConMan ComeOnConMan linked an issue Mar 1, 2024 that may be closed by this pull request
@mustafa-tariqk
Copy link
Owner

Gonna add a couple requirements here:

  • use jsdoc to automatically generate code documentation in client/docs (look at docpy.sh in scripts to see how I did it for python)
  • do NOT make calls to 127.0.0.1 in code, instead, read in a SERVER_URL from .env file. So in steps this is, 1. abstract all calls to 127.0.0.1 and use globalurl + endpoint when making fetch requests, 2. Having the global url read .env with https://github.com/motdotla/dotenv

1 is optional, you should do it if you want a good grade, I mean we said we were gonna make code docs in presentations + we mention to go there to look in README.

2 is critical, if it doesn't get done I have to do extra work (editing all instances of 127.0.0.1 to another url/ip) every time I want the server up and running.

Copy link
Collaborator

@BasicallyOk BasicallyOk left a comment

Choose a reason for hiding this comment

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

Need to address @mustafa-tariqk 's comment

@BasicallyOk
Copy link
Collaborator

Some more information, dotenv does not work in React out of the box because the browser cannot read the local dotenv file. So it takes some work baking it into the webpack first.

Copy link
Collaborator

@BasicallyOk BasicallyOk left a comment

Choose a reason for hiding this comment

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

Merge in main then fix based on the comments

client/src/pages/Complete.jsx Show resolved Hide resolved
Copy link
Collaborator

@BasicallyOk BasicallyOk left a comment

Choose a reason for hiding this comment

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

I changed it myself

@BasicallyOk BasicallyOk merged commit e277cdf into main Mar 4, 2024
1 check passed
@BasicallyOk BasicallyOk deleted the 61-front-end-pages-styling branch March 4, 2024 19:24
@mustafa-tariqk mustafa-tariqk restored the 61-front-end-pages-styling branch March 6, 2024 19:47
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.

Front-end Page Creation
3 participants