Monorepo for Tangled โ€” https://tangled.org

appview/state: Move /knots & /spindles under /settings #660

merged
opened by 5jiji.com targeting master from 5jiji.com/tangled-core: master

Closes https://tangled.org/@tangled.org/core/issues/248

I hope this go code is fine, because i have no damn clue how go works, so i just tried something and it worked If there is a better solution, i would like to know it!

NB: (I've checked if it compiled and worked, by removing commit '3ebdcd: knotserver: improve the logging situation', as for some reason, that commit is uncompilable (even your spindle is complaining))

0
by 5jiji.com 3 comments
expand 3 commits
appview/state: Moved /knots & /spindle to /settings
appview/pages, docs: Update knots links
appview/pages, docs: Update spindle links

thanks for the contribution! some high level thoughts on this work:

  • the original intent of the issue (it is not the clearest about this unfortunately) is to move the knots and spindles pages to be new tabs under the settings page
  • the code here seems to move the routers, which is a good chunk of this work, we also want to make UI changes here to completely move the pages
  • i personally do not think the redirect is necessary, we can move the router directly under settings, without using a redirect

For the second point, does it mean that the links from the user submenu (when you click on your account in the topbar) will be removed, or will stay?

yes, we will remove the links from there. other areas of the UI can be later improved to indicate the existence of selfhosting (the new repo page can contain a link to selfhosted knots, the pipelienes page can have a link to spindles and selfhosting docs etc.)

sign up or login to add to the discussion
1
by 5jiji.com 1 comment
expand 8 commits
appview/state: Moved /knots & /spindle to /settings
appview/pages, docs: Update knots links
appview/pages, docs: Update spindle links
appview/pages: Remove links from dropdown
appview/state: Remove Redirects
appview/{pages,knots}: Update knots page to be more settings like
appview/{pages,spindle}: Update knots page to be more settings like
appview/{knots,settings,spindle}: Added icons for knots & spindle

Hello there, finally could resubmit my patch (as i couldn't do that for some reason), so to update on what i updated:

  • Moved the knots & spindle pages to the settings page
    • I know that i currently have duplicated information, for the icons to be showed, but i have no clue on how to share that information (i still don't know go very much)
    • I tried making the knots & spindle pages fit in, but i have no idea if that's good enough or not
  • Removed the knots & spindle shortcuts from the user dropdown
    • Did not add them on the repo create page & spindle pages, as i don't really know how you'd like them to be set as

And i think that's about it! I may have forgotten a bit of what i did, as it's now been a while since i wrote all the stuff for the patch, as i couldn't resubmit, but i think i touched on all main points.

sign up or login to add to the discussion
2
by 5jiji.com 3 comments
expand 9 commits
appview/state: Moved /knots & /spindle to /settings
appview/pages, docs: Update knots links
appview/pages, docs: Update spindle links
appview/pages: Remove links from dropdown
appview/state: Remove Redirects
appview/{pages,knots}: Update knots page to be more settings like
appview/{pages,spindle}: Update knots page to be more settings like
appview/{knots,settings,spindle}: Added icons for knots & spindle
appview/{knots,pages,state}: formating

Resubmitted to format the code properly!

Generally lgtm! Can you squash the commits into single commit?

nvm I'll merge this PR and squash manually. Thank you for your contribution!

pull request successfully merged
sign up or login to add to the discussion
Labels

None yet.

assignee

None yet.

Participants 3
AT URI
at://did:plc:uakrniojtddxh2juse64bgcm/sh.tangled.repo.pull/3m2z5niwb2y22