gorb/frontend
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Implement settings page #5

Closed
justtemmie wants to merge 0 commits from settings-page into main
pull from: settings-page
merge into: gorb:main
Conversation 5 Commits - Files changed -
justtemmie commented 2025-06-02 11:15:51 +00:00
Owner
Copy link

Implements account settings, currently only accessible by going to /settings manually

i also updated some api calls here and there due to them being out of date, and added a favicon!

Implements account settings, currently only accessible by going to /settings manually i also updated some api calls here and there due to them being out of date, and added a favicon!
justtemmie added 16 commits 2025-06-02 11:15:51 +00:00
feat: Fundementals for settings pages
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
5012517e9b
feat: button components
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
705b37fa06
feat: start of "My Account" page, need API help
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
39fb0a9eab
feat: change colour of focused settings category
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
162ca6833f
feat: log out button should work?
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
fc266ffcc3
feat: basic user popup implemented
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
a2c04af8ce
feat: profile page, EXCEPT FUCKING AVATARS AAAAAHHH
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
622abc9155
feat: fucking explode i hate this
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
0ddddd210e
feat: define expected types for pfp input element
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
a8e8c6b2ef
chore: finnish merge
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
ci/woodpecker/pr/build-and-publish Pipeline was successful
Details
a38589615b
sauceyred added 1 commit 2025-06-03 20:09:28 +00:00
feat: make profile settings headings into block labels for accessibility
All checks were successful
ci/woodpecker/push/build-and-publish Pipeline was successful
Details
ci/woodpecker/pr/build-and-publish Pipeline was successful
Details
22b43cde79
sauceyred requested changes 2025-06-03 20:42:55 +00:00
Dismissed
sauceyred left a comment
Owner
Copy link

Some notes to supplement the comments:

  • Use relative units rather than absolute units like px
    • elements: dvh (dynamic view height), dvw (dynamic view width), or %
    • font-size: em (font-size relative to parent) or rem (font-size relative to root)
  • Avoid using heading (<h1-6>) elements for things that aren't titles or section labels, prefer using non-semantic elements like <p> combined with the font-size CSS property for purely visual design, headings are semantic elements primarily used for accessibility, and having 5 of them in a row isn't great
  • Use camelCase for variables, functions, and methods; PascalCase for classes and component folders and files; and snake-case for other folder names, for consistency
  • The font sizes on the section titles and buttons on the left of the settings are way too big imo

Re-request review once you've made these changes, or let me know if you disagree with anything.

Some notes to supplement the comments: - Use relative units rather than absolute units like px - elements: dvh (dynamic view height), dvw (dynamic view width), or % - font-size: em (font-size relative to parent) or rem (font-size relative to root) - Avoid using heading (`<h1-6>`) elements for things that aren't titles or section labels, prefer using non-semantic elements like `<p>` combined with the `font-size` CSS property for purely visual design, headings are semantic elements primarily used for accessibility, and having 5 of them in a row isn't great - Use camelCase for variables, functions, and methods; PascalCase for classes and component folders and files; and snake-case for other folder names, for consistency - The font sizes on the section titles and buttons on the left of the settings are way too big imo Re-request review once you've made these changes, or let me know if you disagree with anything.
components/Userpopup.vue Outdated
@ -0,0 +7,4 @@
<strong>{{ props.user.display_name || "display_name" }}</strong>
</p>
<p id="username-and-pronouns">
{{ props.user.username || "username" }} - {{ props.user.pronouns || "un/defined" }}
sauceyred commented 2025-06-03 20:13:01 +00:00
Owner
Copy link

Unnecessary fallback string, username can't be null

Unnecessary fallback string, username can't be null
justtemmie marked this conversation as resolved
components/Userpopup.vue Outdated
@ -0,0 +22,4 @@
const { fetchMembers } = useApi();
const props = defineProps<{
user: UserResponse | any, // actually UserResponse | null but TS is yelling at me again
sauceyred commented 2025-06-03 20:13:48 +00:00
Owner
Copy link

| any isn't actually necessary, not sure why TS would be yelling at you, doesn't for me

`| any` isn't actually necessary, not sure why TS would be yelling at you, doesn't for me
justtemmie marked this conversation as resolved
components/settings/user_settings/Account.vue Outdated
@ -0,0 +26,4 @@
</div>
<!-- i love html -->
<br>
sauceyred commented 2025-06-03 20:23:56 +00:00
Owner
Copy link

Prefer using margin properties for spacing in layout, use <br> for line breaks in text/paragraphs

Prefer using `margin` properties for spacing in layout, use `<br>` for line breaks in text/paragraphs
justtemmie marked this conversation as resolved
components/settings/user_settings/Account.vue Outdated
@ -0,0 +93,4 @@
input.type = 'file';
input.accept = 'image/*';
input.onchange = async (e) => {
sauceyred commented 2025-06-03 20:25:32 +00:00
Owner
Copy link

Prefer addEventListener rather than .on{event} for consistency with the rest of the codebase, in this case input.addEventListener("change", (e: Event) => {})

Prefer `addEventListener` rather than `.on{event}` for consistency with the rest of the codebase, in this case `input.addEventListener("change", (e: Event) => {})`
justtemmie marked this conversation as resolved
pages/settings.vue Outdated
@ -0,0 +158,4 @@
}
</style>
<!-- not scoped, these are used by children] -->
sauceyred commented 2025-06-03 20:34:25 +00:00
Owner
Copy link

Rather than making a global style, use :deep(), which should work for children of components, e.g.:

:deep(h5) {
    color: red;
}
Rather than making a global style, use `:deep()`, which should work for children of components, e.g.: ```css :deep(h5) { color: red; } ```
justtemmie marked this conversation as resolved
sauceyred added the
Kind/Feature
 label 2025-06-03 20:47:43 +00:00
justtemmie added 1 commit 2025-06-11 15:42:40 +00:00
justtemmie commented 2025-06-11 15:44:12 +00:00
Author
Owner
Copy link

i've decided to not use dynamic units for the user popup, i need help deciding how to do stuff, and it can be done later.

border-radius is still using pixels, i think relative units just look bad for them.

otherwise i believe everything's been "fixed".

i've decided to **not** use dynamic units for the user popup, i need help deciding how to do stuff, and it can be done later. border-radius is still using pixels, i think relative units just look bad for them. otherwise i believe everything's been "fixed".
requested review from sauceyred 2025-06-11 15:45:13 +00:00
radical added 1 commit 2025-06-23 18:02:36 +00:00 
co-authored-by: JustTemmie <git@beaver.mom>
radical added 1 commit 2025-06-23 18:48:00 +00:00 
co-authored-by: JustTemmie <git@beaver.mom>
co-authored-by: SauceyRed <saucey@saucey.red>
radical commented 2025-06-23 18:50:11 +00:00
Owner
Copy link

still have to fix the user-popup thing, but we'll fix that later

  • JustTemmie
still have to fix the user-popup thing, but we'll fix that later - JustTemmie
sauceyred approved these changes 2025-06-23 18:50:34 +00:00
sauceyred left a comment
Owner
Copy link

Mostly fine, will make some minor changes later, but fine to push.

Mostly fine, will make some minor changes later, but fine to push.
radical commented 2025-06-24 12:31:04 +00:00
Owner
Copy link

Merged manually

Merged manually
radical closed this pull request 2025-06-24 12:31:04 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
sauceyred
Labels
Clear labels   
Compat/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: gorb/frontend#5
No description provided.
Delete branch "settings-page"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?