feat(gui, config): support simple folder grouping (fixes #2070)#10563
feat(gui, config): support simple folder grouping (fixes #2070)#10563calmh merged 9 commits intosyncthing:mainfrom
Conversation
|
Why should this affect the protocol? IMHO folder grouping is a local thing to organize the device's individual folder list. Groups may not mean the same to other users. |
|
This can be even more simple, and can be fully a UI feature with no changes at all. g1/f1 ends up as: g1 g2 etc |
|
That idea has been discussed before on the forum and in some issues, e.g. #2070. The major downside is that it fails for some common labels where a slash is used differently. For example: A separate config field for the grouping was the favored approach IIRC. Heuristics such as separation levels by a slash can be applied there. But mixing semantics with the label field will cause trouble for existing setups. |
No probs. I can drop that. I was just following something you said here about adjusting the protocol but I might have misinterpreted it #2070 (comment) |
This can be fixed by allowing users to mask e.g |
912fe37 to
93aa32a
Compare
93aa32a to
e51b75e
Compare
Really? That still leaves users needing to bend over backwards to adjust their labels, when they simply do not need a grouping feature. Label is a label, completely free form, already used that way. Semantics can be attached to an additional field, with a special purpose.
That was about protobuf, which we had used to define the configuration structure at the time. I believe we went back to simple go structs since. |
|
I would also like to suggest keeping labels their own thing. For example, all my labels are basically relative folder paths (separated with @maen-bn Could you post a screenshot with more folders in each group than just one? I would also be curious how the UI looked if you dropped the group background colour (as I think the current version doesn't look the best, especially with the empty space below the last folder). Alternatively, maybe drop the background for ungrouped folders only? If possible, it would also be nice to see how everything looks in the light and black themes 🙂. |
|
@acolomb Ah okay that makes sense, appreciate the context given I'm a newbie to contributing here. @tomasz1986 there is a part in the gif where two folders are in the same group But I can get a screenshot where there's more than two as well if that helps. Yeah I can remove the background for them and show some screenshots for the light theme too |
|
Here you go @tomasz1986, I've removed the background for the groups and examples of more folders grouped with all the themes available in the GUI Light
Dark
Black
|
|
Thanks for the screenshots! Not sure what others think, but for me, the current iteration (without the background) looks better 🙂. |
|
Loving the look of this |
|
Looks reasonable to me, but the immediate followup question will be why this is not supported for devices. It seems reasonable to support the same mechanism for both at the same time, possibly? |
Yeah that's very reasonable. I'm happy to sort that out in separate PR if that's cool |
|
I think it would be fine to have in the same, as a single grouping concept. |
|
Please also push a relevant PR to the Docs, so that https://docs.syncthing.net/users/config#config-option-folder.group works (and the same for device groups, if implemented) 🙂. |
Document upcoming fuctionality addtion for grouping done in syncthing/syncthing#10563 Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
|
Okay @calmh I've popped in some new changes to add grouping to devices too. Here some screenshots (redacted info as it was my own data) showing the new changes
Also (FYI @tomasz1986 ) I have stuck in changes on the docs repo to cover the changes here which can be found in the PR syncthing/docs#1001 Let me know if I need to adjust anything |
| console.log("HELLO!!!"); | ||
| console.log($scope.deviceGroups); |
| <p translate ng-if="currentDevice.deviceID == myID" class="help-block">Shown instead of Device ID in the cluster status. Will be advertised to other devices as an optional default name.</p> | ||
| <p translate ng-if="currentDevice.deviceID != myID" class="help-block">Shown instead of Device ID in the cluster status. Will be updated to the name the device advertises if left empty.</p> | ||
| </div> | ||
| <div class="form-group" ng-class="{'has-error': deviceEditor.deviceGroup.$invalid && deviceEditor.folderGroup.$dirty && !editingDeviceDefaults()}"> |
lib/config/folderconfiguration.go
Outdated
| Path string `json:"path" xml:"path,attr"` | ||
| Type FolderType `json:"type" xml:"type,attr"` | ||
| Devices []FolderDeviceConfiguration `json:"devices" xml:"device"` | ||
| Group string `json:"group" xml:"group,attr"` |
There was a problem hiding this comment.
| Group string `json:"group" xml:"group,attr"` | |
| Group string `json:"group" xml:"group,attr,omitempty"` |
gui/default/index.html
Outdated
| <span ng-if="folder.versioning.type == 'simple'" tooltip data-original-title="{{'Keep Versions' | translate}}"> | ||
|  <span class="fa fa-file-archive-o"></span> {{folder.versioning.params.keep}} | ||
| <div ng-repeat="(folderGroupKey, folderGroup) in folderGroups"> | ||
| <h4>{{ folderGroup }} |
There was a problem hiding this comment.
Might want to make the h4 conditional on the group name not being empty ""?
| } | ||
| }; | ||
|
|
||
| $scope.folderListByGroup = function (group) { |
There was a problem hiding this comment.
These are called several times per frame, perhaps it would be worth it to cache them as a map/dictionary instead?
a7a1e11 to
855ca38
Compare
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
All sorted @calmh and changed up how the folder and device groups are constructed so rather than just a list of groups they're now a map with group as the key and their related devices/folders so no need for those filtered function that were being called over and over again. |
Document upcoming fuctionality addtion for grouping done in syncthing/syncthing#10563 Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
…solves syncthing#2070) Rather than filtering a list of devices and folders by group name which can be called multiple times per frame, the change here maps the devices and folders into their groups only when the local config changes and makes it easier to access just by keying into the map. Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
lib/config/folderconfiguration.go
Outdated
| Path string `json:"path" xml:"path,attr"` | ||
| Type FolderType `json:"type" xml:"type,attr"` | ||
| Devices []FolderDeviceConfiguration `json:"devices" xml:"device"` | ||
| Group string `json:"group" xml:"group,attr,omitempty"` |
There was a problem hiding this comment.
| Group string `json:"group" xml:"group,attr,omitempty"` | |
| Group string `json:"group" xml:"group,attr,omitempty" restart:"false"` |
gui/default/index.html
Outdated
| <span class="inline-icon"> | ||
| <span ng-class="rdConnTypeIcon(rdConnType(deviceCfg.deviceID))" class="reception reception-theme"></span> | ||
| </span> | ||
| <div ng-repeat="(deviceGroupName, devices) in devicesGrouped"> |
There was a problem hiding this comment.
This clobbers $scope.devices inside the loop, should probably use a more specific name... (Same for the folders I think)
gui/default/index.html
Outdated
|  <span class="fa fa-file-archive-o"></span> {{folder.versioning.params.keep}} | ||
| <div ng-repeat="(folderGroupName, folders) in foldersGrouped"> | ||
| <h4 ng-if="folderGroupName !== ''">{{ folderGroupName }} | ||
| <span ng-if="folders.length > 1 && folders.length > 0"> ({{folders.length}})</span> |
There was a problem hiding this comment.
Sorry typo that's suppose to be checking how many folders there are and if the group has a name that isn't empty
…ng#2070) Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
…esolves syncthing#2070) Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
…) (syncthing#10563) Adds a very simple way to group up folders to help with organising from the GUI. Signed-off-by: Ben Norcombe <bennorcombe@pm.me> Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Document upcoming fuctionality addtion for grouping done in syncthing/syncthing#10563 --------- Signed-off-by: Ben Norcombe <bennorcombe@pm.me> Signed-off-by: Jakob Borg <jakob@kastelo.net> Co-authored-by: Jakob Borg <jakob@kastelo.net>
Regression was introduced in PR syncthing#10563 due to new devices grouping feature not utlising the otherDevices utility function to ensure the local device is not shown in the remote devices list Signed-off-by: Ben Norcombe <bennorcombe@pm.me>
Ah yeah apologies. The |








Purpose
Adds a very simple way to group up folders to help with organizing from the GUI. My intention is to not make this too complicated and just as a starter but potential follow on PRs to improve this but out of scope here are:
Resolves #2070
Testing
Screenshots
Documentation
I haven't done this yet as I want to wait for the feedback on the changes. I'm happy to do those doc changes too but the changes to the doc should be
Authorship
Ben Norcombe bennorcombe@pm.me