feat: server folders in the server list - #1589
GalaxyAkiii wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Excellent first draft. Needs a few changes, but overall I'm happy with how this looks code-wise. Do note that in order to merge this we have to make sure the android client supports it (or atleast doesn't stomp the folders).
This change will likely need buy-in from the core team, so expect merging this to take a little bit.
I believe in it's current state it would not pass UI review, and that is primarily due to the folders not showing their contents when collapsed. I left a comment regarding this.
| const id = `${FOLDER_PREFIX}${Date.now().toString(36)}${Math.random() | ||
| .toString(36) | ||
| .slice(2, 8)}`; |
There was a problem hiding this comment.
We have access to ULID generation in the client via the ulid package.
import { ulid } from "ulid";
const id = `${FOLDER_PREFIX}${ulid()}`;| fallback={entry.item.name} | ||
| interactive | ||
| <Switch> | ||
| <Match when={entry.item.type === "server" && entry.item}> |
There was a problem hiding this comment.
Shouldn't entry.item be before the equality check here? Otherwise if it's falsy the first check might error.
| </Tooltip> | ||
| )} | ||
| </Match> | ||
| <Match when={entry.item.type === "folder" && entry.item}> |
There was a problem hiding this comment.
Shouldn't entry.item be before the equality check here? Otherwise if it's falsy the first check might error.
| <a onClick={() => state.ordering.toggleFolder(props.entry.folder.id)}> | ||
| <Avatar | ||
| size={42} | ||
| holepunch={collapsed() && mentions() ? "top-right" : "none"} | ||
| overlay={ | ||
| <Show when={collapsed() && mentions()}> | ||
| <Unreads.Graphic count={mentions()} unread /> | ||
| </Show> | ||
| } | ||
| fallback={<MdFolder />} | ||
| /> | ||
| </a> |
There was a problem hiding this comment.
Rather than falling back to the MdFolder icon, could it display the icons of the servers inside? This may require making a new container to hold avatars in a smaller state, but I think it will make it more clear what's in each folder if it shows them.
|
To make dnd work you will need to:
It's possible I may be missing something with this, but this should be it. This may be a tall request, so let me know if you run into any issues and I can take a look at it and help implement it. |
5798782 to
a95f3a0
Compare
|
Hey @Dadadah, thanks for the feedback. Couldn't sleep last night (or really tonight so far) so ended up staying up overnight hacking away at this even before you left your comment and all of yesterday before practice. A good chunk of what you brought up was actually stuff I had started looking into right after putting up the initial PR, so I'm glad to say it's pretty much all sorted now. I do apologise, by my own standards this was a half done job that shouldn't have been put out in the first place. Regarding the Android client, that's a really great point about not stomping over folders, thank you for bringing it up. Having tinkered with and tried developing an alternative iOS client in the past (sitting in the ether for now), I know the client codebases fairly well and went back to check specifically from your point today. If folders were stored directly inside On the drag-and-drop side, I actually started out trying to adapt I ended up writing a dedicated drag handler for the server rail ( I also added the mini-grid preview on collapsed folders so you can see avatars of what's inside instead of just a generic folder icon. You'll also notice Video of Folders (File was too big for GitHub) Mix of collapsed and coloured folders That should be all from me. Definitely understand the need for patience in this. Please let me know if you need anything further from me. I have tried to go over everything, but if I have missed anything at all please do let me know. |
|
Please drop the i18n folder from this pr. This PR grew in scope a bit so I'll sit down and review it some time over the next few days. Thanks for the write up! |
Signed-off-by: Aki <aki@akiworks.xyz>
a95f3a0 to
73c337d
Compare
|
All done, Thank you. |
|
I've spoken with the core team about this, and it's come to our attention that we are unable to modify the existing schema of synced store keys. This means that for this pr, it will need to include a migration from the |
|
|
||
| const NoColourIndicator = styled(ColourIndicator, { | ||
| base: { | ||
| border: "2px solid var(--md-sys-color-outline)", |
|
Fair enough, I will continue working on this tonight or tomorrow on the flight. Thank you both. |
|
@Dadadah checking this still applies, since the storage changed on the 9th. Folders no longer live inside What the PR does still do is write to Two ways I can read that:
Which did you have in mind? And is a separate synced key okay at all, or should all of this sit under (@infi's dashed border I'll pick up in the same pass.) |
amycatgirl
left a comment
There was a problem hiding this comment.
I was testing this locally and ran into an issue when dragging servers around with the custom draggable implementation. Left a comment about it.
| <Show when={isInsertionBefore(props.drag, props.server.id)}> | ||
| <div class={railInsertion} /> | ||
| </Show> | ||
| <a href={state.layout.getLastActiveServerPath(props.server.id)}> |
There was a problem hiding this comment.
This anchor needs to have the draggable="false" attribute to prevent it from taking precedence over the draggable container.
| fallback={ | ||
| <Show | ||
| when={collapsed() && props.entry.servers.length} | ||
| fallback={<MdFolder />} |
There was a problem hiding this comment.
Nit: Wouldn't it make more sense to use FolderOpen here?
Preferably, this would use the much clearer folder_open icon from material symbols, but this could be done in a separate PR.
| style={{ | ||
| // a box shadow rather than an outline, so that the unread | ||
| // badge still draws on top of it | ||
| "box-shadow": |
There was a problem hiding this comment.
Nit: I should probably leave this for UX Review, but I think that an outline looks out of place with the rest of the UI.
Maybe a background using the colour plus some transparency on top of it could be better? Like when the folder is expanded.
…rSidebar Signed-off-by: Aki <aki@akiworks.xyz>
|
Went with option 2 for the migration, keeping Summary of where it stands:
A folder's members are kept in
That leaves the question of whether a second synced key is acceptable. If the core team would rather everything sat under Also in this pass:
|
amycatgirl
left a comment
There was a problem hiding this comment.
Seems to work completely fine now.
Left a comment about the server folder background while not expanded since there seemed to be a misunderstanding between what I meant and the code I highlighted in the review.
But again, UX review is not my thing, you can feel free to ignore my comment since it's better to leave this to Jennifer.
LGTM
| <FolderGroup | ||
| expanded={!collapsed()} | ||
| style={{ | ||
| background: props.entry.folder.colour |
There was a problem hiding this comment.
Fair enough, I will take that into consideration and do it as well although I will await UX review and go with which one is selected. Thank you.






Adds collapsible folder grouping to the server list sidebar.
Resolves #721
Addresses stoatchat/stoatchat#577 (and duplicate #933)
What changes
Ordering.ts):string[]to(string | ServerFolder)[].SynchronisedStores, so folders sync across devices with zero backend/schema changes.clean()handles deduping and migration when dealing with flat arrays written by older clients.orderedServers()still returns a flattened array with folder items inlined so keyboard shortcuts, navigation, and the emoji picker don't break.orderedEntries()accessor for the nested layout.ServerList.tsx):ServerEntryfromServerList.FolderEntrywith expand/collapse toggle and aggregated unread/mention counts when folded.FolderGroupwith a theme-awarecolor-mix(...)background so the expanded rail stands out cleanly across themes.ServerContextMenu.ServerFolderContextMenuto rename or unpack/delete folders.EditServerFoldermodal for renaming.Notes & caveats
solid-dnd-directivesetup runs on a single flat container. Teaching it nested drop zones is a much bigger refactor, so adding/removing from folders is context-menu only for now.clean()step strips non-string IDs. Server order survives, but folders get unpacked back to flat entries on their write.How was this PR tested?
color-mixsurface contrast across dark, light, and high-contrast themespnpm exec tsc --noEmitclean across all feature filespnpm exec eslintandpnpm exec prettier --checkclean (0 errors, 0 warnings across all 10 files)Screenshots & Screencasts (if appropriate)
Create Folder/Add Server to existing folder






Folder in server list open
Rename Folder
Folder Management
Folder Closed
Checklist:
Please declare, if any, LLM usage involved in creating this PR
None