old
/
LinkList
2
0
Fork 0
Code Issues 3 Pull Requests Projects Releases Wiki Activity
Labels Milestones

Hookup update links endpoint #27

Merged
q63xu merged 0 commits from chore/update-links into main 2 years ago
Conversation 11 Commits N/A Files Changed N/A
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/

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.
Reviewers
Request review
No reviewers
Labels
Apply labels
Clear labels
deployment
No Label deployment
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: old/LinkList#27
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'chore/update-links'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes