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

implementation of router, navigation bar and settings. #38

Open
wants to merge 56 commits into
base: development_2.0.0
Choose a base branch
from

Conversation

abdahmed22
Copy link

@abdahmed22 abdahmed22 commented Jul 23, 2024

Description

Create routes for the website and Create a navigation bar to use the router.

Changes

  • Change favicon & title
  • Create routes
  • Create navigation bar
  • create settings page and forms

Related Issues

client/src/api/axios.ts Outdated Show resolved Hide resolved
client/src/components/settings/ProfileInformationForm.vue Outdated Show resolved Hide resolved
client/src/components/settings/SecurityForm.vue Outdated Show resolved Hide resolved
@abdahmed22 abdahmed22 requested a review from maayarosama July 31, 2024 10:19
client/src/utilities/validators.ts Outdated Show resolved Hide resolved
client/src/utilities/validators.ts Show resolved Hide resolved
client/src/utilities/validators.ts Outdated Show resolved Hide resolved
client/src/utilities/validators.ts Outdated Show resolved Hide resolved
client/src/pages/DashboardView.vue Outdated Show resolved Hide resolved
client/src/layouts/NavigationBar.vue Outdated Show resolved Hide resolved
client/src/layouts/NavigationBar.vue Outdated Show resolved Hide resolved
client/src/layouts/NavigationBar.vue Outdated Show resolved Hide resolved
client/src/layouts/NavigationBar.vue Outdated Show resolved Hide resolved
client/src/layouts/NavigationBar.vue Outdated Show resolved Hide resolved
@zaelgohary
Copy link

Branch name should be 'base branch + a descriptive name for changes done in the PR'. Change branch name to development_2.0_add_routes

@ehab-hassan
Copy link
Collaborator

testtracker_logo
we need to add logo at the nav

@maayarosama maayarosama requested a review from zaelgohary August 4, 2024 09:14
@abdahmed22 abdahmed22 closed this Aug 4, 2024
@abdahmed22 abdahmed22 deleted the Abdelrahman_development branch August 4, 2024 11:02
@abdahmed22 abdahmed22 restored the Abdelrahman_development branch August 4, 2024 11:07
@abdahmed22 abdahmed22 reopened this Aug 4, 2024
@abdahmed22 abdahmed22 marked this pull request as draft August 4, 2024 11:36
@abdahmed22 abdahmed22 changed the title implementation of router and navigation bar. implementation of router, navigation bar and settings. Aug 4, 2024
@abdahmed22 abdahmed22 marked this pull request as ready for review August 4, 2024 12:34
Copy link

@zaelgohary zaelgohary left a comment

Choose a reason for hiding this comment

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

Good job, just small comments.

  • Settings title should be aligned with page content
  • check if old pass in settings is true
  • change settings fs to 1.7rem
  • add :disabled="!form?.isValid || loading" to all forms

@abdahmed22 abdahmed22 requested a review from zaelgohary August 22, 2024 12:13
@Mahmoud-Emad
Copy link
Collaborator

Mahmoud-Emad commented Aug 22, 2024

Good job ya 3obad, i just have some comments

  • Set the public information tab as the active tab on mounting the settings page

image

  • Change the color of the change password button to be visible, I thought it was disabled even though I have a valid form. also try to reduce the size and move it to the right

image

  • The same goes for the first form

image

  • Optimize the phone number input validation, it accepts numbers even if the length is 1

image

  • Let's center the forms

image

  • The logo should be clickable and should navigate to the Dashboard on click, I know that we have a Dashboard link beside the logo, but usually, we make the icon of the site clickable

image

@Mahmoud-Emad
Copy link
Collaborator

Mahmoud-Emad commented Aug 27, 2024

Why Name must be at least 5 characters., it could be Sam, Emad, Wael, or Taha BTW

  • image

it shouldn't accept social characters btw

Screencast.from.08-27-2024.10.30.08.AM.webm

After I pressed Submit I got an error then I noticed that the loading spinner was loading forever

Screencast.from.08-27-2024.10.32.40.AM.webm

When the users press submit, you have to send their access_token with the request, to fix the Permission issue
Here are the server logs

System check identified no issues (0 silenced).
August 27, 2024 - 07:34:41
Django version 4.0.4, using settings 'settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
Requested user is: AnonymousUser

Also, I should be able to update the email if there is no email associated

  • image

Same happening in the Passwords form, The loading button was loading forever, the request API URL wasn't valid

  • image

@Mahmoud-Emad
Copy link
Collaborator

Mahmoud-Emad commented Sep 1, 2024

I got this error when navigated to the settings page, I think we need to check first if there TEST_TRACKER_ACCESS_TOKEN then take action, if not, we need to redirect the user to the login

image

Also, please merge the latest changes to support the auth feature

@abdahmed22 abdahmed22 marked this pull request as draft September 8, 2024 08:36
@abdahmed22 abdahmed22 marked this pull request as ready for review September 16, 2024 09:28
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.

5 participants