Hookup update links endpoint #27

Merged
q63xu merged 0 commits from chore/update-links into main 2 years ago
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner
There is no content yet.
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

Add submit and cancel buttons for updating links @l42luo feel free to take a look to see how links are being updated!

Add submit and cancel buttons for updating links @l42luo feel free to take a look to see how links are being updated!
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

@a3thakra I combined both LoggedInState and LoggedOutState into AuthState even though it makes sense to only expose login/logout functionality when appropriate, conditionally rendering context values makes it confusing imo and hard to work with 😅

@a3thakra I combined both `LoggedInState` and `LoggedOutState` into `AuthState` even though it makes sense to only expose login/logout functionality when appropriate, conditionally rendering context values makes it confusing imo and hard to work with :sweat_smile:
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

@a3thakra I got rid of loggedIn as a separate state variable and made it dependent on headers so we don't have to worry about updating two similar state variables everytime

@a3thakra I got rid of `loggedIn` as a separate state variable and made it dependent on `headers` so we don't have to worry about updating two similar state variables everytime
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

@a258wang I combined all ur components into one, I really liked the approach you were taking by breaking things down into separate component, including the separation between LoginHead and LoginBox and identifying how LoginHead won't have to be re-rendered when the state in LoginBox changes!

But making components too fragmented can reduce readability, for example there was a LoginScreen component in pages/editor.tsx which is a purely presentational component that should only live with other Login subcomponents. Also, with fragmenting components you would have the overhead of naming components such as Login{Head/Box/screen} this might also make it confusing to trace for others

Overall really good job on identify when u should break components up, just giving you a heads up since a lot of frontend codebases out there heavily values readability over minor performance benefits

@a258wang I combined all ur components into one, I really liked the approach you were taking by breaking things down into separate component, including the separation between `LoginHead` and `LoginBox` and identifying how `LoginHead` won't have to be re-rendered when the state in `LoginBox` changes! But making components too fragmented can reduce readability, for example there was a `LoginScreen` component in `pages/editor.tsx` which is a purely presentational component that should only live with other `Login` subcomponents. Also, with fragmenting components you would have the overhead of naming components such as `Login{Head/Box/screen}` this might also make it confusing to trace for others Overall really good job on identify when u should break components up, just giving you a heads up since a lot of frontend codebases out there heavily values readability over minor performance benefits
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner
Revalidates the page every 60s using ISR https://blog.logrocket.com/incremental-static-regeneration-with-next-js/
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

Remove unused tailwind styles

Remove unused tailwind styles
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

@a3thakra changed to fetching from an endpoint rather than a links.json file!

@a3thakra changed to fetching from an endpoint rather than a `links.json` file!
a3thakra commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

Use an environment variable instead. Something like API_HOST=http://localhost:5000

Use an environment variable instead. Something like API_HOST=http://localhost:5000
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

changed this line in version 2 of the diff

changed this line in [version 2 of the diff](/csc/linktree/-/merge_requests/22/diffs?diff_id=52847&start_sha=4870c20697f79ba0ccfd77fd6e2bda38af9abaec#35c38fc95bc6dcaa0b028403beb5b2e31bed6870_6_6)
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

added 1 commit

  • 8520bc7a - Hookup clicks endpoint

Compare with previous version

added 1 commit <ul><li>8520bc7a - Hookup clicks endpoint</li></ul> [Compare with previous version](/csc/linktree/-/merge_requests/22/diffs?diff_id=52847&start_sha=4870c20697f79ba0ccfd77fd6e2bda38af9abaec)
q63xu commented 2 years ago (Migrated from git.uwaterloo.ca)
Owner

added 2 commits

  • c17dff17 - 1 commit from branch main
  • 13310d07 - Merge branch 'main' into chore/update-links

Compare with previous version

added 2 commits <ul><li>c17dff17 - 1 commit from branch <code>main</code></li><li>13310d07 - Merge branch &#39;main&#39; into chore/update-links</li></ul> [Compare with previous version](/csc/linktree/-/merge_requests/22/diffs?diff_id=52848&start_sha=8520bc7aac8e5830c0871560ee90722e44993d1e)
The pull request has been merged.
Sign in to join this conversation.
No reviewers
No Label deployment
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: old/LinkList#27
Loading…
There is no content yet.