Shared-with settings implementation

Hi

This is about the enhancement 8972. To keep the GUI (Folders pane) coherent with Reduced by ignore patterns, there should not be a link to the target device config here (Although, there must be discussion around this PoV). Instead the whole list should be a single link to the Edit Folder/Sharing tab. Furthermore, the way the enhancement is implemented misses the “quick setting” aim when it comes to add a new member to the cluster. IIUC this is not the way @Paul-Stern described the feature request.

[EDIT]: @Paul-Stern is not the OPoster. Instead it was @BartG95

Sorry, I don’t follow. What do you think is wrong with that change?

I hope I’m not as nasty as Audrius about new features in the GUI :joy:

I think this one misses its quick access goal. When the idea was good (IMO) when first proposed, I feel the way it is built is not efficient at all. Just give a try. Click on a member, you won’t go to de Folder’s Sharing tab. Instead you will go to the last opened tab in device config (not even the “Sharing” tab - which one whatever wouldn’t allow to add a new member to the cluster… because we are in the list of Folders this device (the clicked one) already belongs to).

[EDIT] : also we miss the counterpart in Devices pane, were a link at each Other devices would be a link to direct access to this very device Sharing tab.

For both side of the GUI an ugly way would be to add the link to the titles “Shared With” (Folders pane) & “Folders” (Devices pane) headings. But it is ugly because it would be the first links on the left side of the panes

Instead the whole content would be the link… as a whole. John Stern proposed that each member would be a link to the Edit Folder/Sharing dialog tab, but this would be, I believe, a bit unintuitive. Another way could be to add a small pen icon on left of each list with a “Edit list” or “Edit membership”/“Edit Clusters list” tooltips.

Yeah, so looking at the issue again, is seems that the original proposal was that clicking on any of the devices in the “Shared With” list should bring up the “Sharing” tab of the currently viewed folder.

The final implementation is very different, i.e. clicking on each device brings up the edit window for the actual device in question.

Honestly, I personally find the current implementation more useful, but it does miss the point of the original issue. On the hand, making the whole “Shared With” list of devices one giant link would look probably rather ugly, especially when it’s many of them in the list.

Accessing the Sharing tab is two clicks away (Edit, Sharing). Do we really need to shorten it to one click?

I see the issue linked with the PR that closed it really does describe a different approach, but is open to interpretation. But I still don’t see which OP you mean describing the original idea. Where? Paul-Stern was just asking whether that issue still existed and showed interest in solving it. It was a different user making the PR and stating it would fix that issue. Since the issue had been left open for a long time and obviously had interest from users, I thought a fix would be welcome and approved the changes after some detail discussion.

As to the utility of this function, well I haven’t ever felt the need for it, but IMHO it could be useful as it is now. Saving just one click by making it a shortcut to the same folder’s Sharing tab is not worth it.

As for the symmetry, it was mentioned in the PR, but I think it’s a good exercise for new contributors, thus didn’t insist on adding the devices side in the same PR.

[EDIT] Okay, I did a fork and try my first contribution…

True

Well… the more I think, the more I wander if we really need this. On one hand we already have the link “Reduced by ignore patterns” which I think is a good way to drive new users to an important nearAdvanced feature, less intimidating. On the other hand, as Audrius said when this was introduced, this makes the GUI something like a cockpit.

Well I didn’t read the “fixes #9472”, sorry. I do it ATM…hmmm quite a long and deep discussion for me-newb as a dev beginner. Let’s study that then come back here.

I agree it’s only a save-one-click. But isn’t it the aim of hypertext? In fact it’s a trade because in the end everything in the GUI would turn to be a link, e.g. Folder Type, File Pull Order and Rescans would link to Advanced tab, as well as File Versioning would link to the File Versioning tab. Notice although that not all items in the folder items is a candidate to a setting in any tab.

Again, it’s worth for anyone interested about this feature request to read the issue #9472… then I go :wink:

Just a quick comment, links are invitations. I saw many user (I even can say all off the ~20 I helped to use Syncthing (private use, not professional)) that do not dare to take ownership on the software. They let it as is, never add a new share for new usage, never tweak Trash retention time, nothing. For this reason, some well choosen items to show as one-click links to the related setting tab in both Folders and Devices GUI chapters may be incentives be happier with ST.

Done. Well… IMHO the GUI takes a wrong turn: editing Devices from within the Folders pane is a totally new paradigm, and (although coherent) André asks Jaspitta to do the same on Devices pane :worried:. I feel this is confusing for new users, breaking the strong difference between Devices and Folders represented by their place in the GUI. But maybe this is just because I’m an old grouch that doesn’t like the taste of coffee changed.

In all honestly (nothing against the work itself and the reviews involved, which is always appreciated and welcome), the PR-handling was done in a very unstructured manner.

I think it would be a good idea to look at Thoughts on code review again and come to a consensus. Because it feels like that topic was made to prevent exactly cases like this…

The reviews instantly dove into the details (nitpicks even). Which is far from optimal at that stage, imo. The PR itself didn’t contain any information other than that it fixes something, so then the first step would be to verify whether the proposed approach actually fixes that, right. But that kind of information was lacking in the PR-content, so to actually be able to review it - a request for a proper structured PR would be in place(?). Then you get at least a bit more insight in the thought behind it.

Then the use of it is rather confusing. Clicking on some entry in the folder-pane to end up in the general tab of a device-specific dialog. I mean…that’s not at all intuitive nor is it very useful to change the share-settings of a specific folder there? (even though it can be changed there too, I don’t think it’s a very common practise for general users).

1 Like

I think the ignore patterns link is for different purpose, e.g. if you want to quickly modify the patterns to sync files on demand, then ignore them again, etc. Overall, it makes the whole process quicker. It can’t really be for new users, because the link is only present there if you already have some ignore patterns present :wink:.

Other than ignore patterns specifically, I don’t think it’s common to edit other folder settings very often… which is likely the reason why there have been no similar links to any of them so far.

Yeah I didn’t really see or follow that PR, but I agree with the OP criticism. I don’t see a reason to have a quick link to the device editor there, rather I expected it to be a quick link to the sharing settings for the folder in question. That’s what I thought it would do…

Good point about the dynamic show yes|no link :wink:. But ~mitigation~, I always use at least basic ignores in Edit Folder Defaults so that…

Also, I have more often used and reverted back to the default “Random” the setting for File Pull Order when I need some files in a hurry.

So did I. I cloned the project and I’m currently studying this html code to propose something in this way. Please let me some time as I’m a real beginner

Yes man. I think I misunderstood what @Jaspitta wanted to do, believing he wanted to do… what I thought :roll_eyes:. Classic error isn’t it? That’s why I said (my bad) his links should target the Folder’s “Sharing” tab, and I thought he forgot to set some pre-existing variable (something like editDevicelastUsedTab in my newb-dev imagination).

My assumption as a reviewer (yes, didn’t look closely enough or the issue description was ambiguous) was that the PR fixes the issue mentioned. And that in turn was based on Paul-Stern having previously shown interest to work on the fix, which I’ve tried to encourage to attract a new contributor. And regarding the review process itself, yes it did quickly focus on details while not looking at the general utility. But that was surely a learning experience in itself: starting out with an elaborate patch for what effectively ended up being a one-liner. Keeping PRs focused without useless whitespace change noise is an important lesson especially for new contributors, IMHO. In that regard, the detail-first discussion was also useful.

So what should the link do if we keep it? For me it was absolutely perfectly clear and expected that clicking the name of a device would open something for that device. Everything else would have been strange to me, that’s why I concluded it was done correctly. I really haven’t wished for it yet, but it is definitely a bigger improvement than saving one click. Seeing a folder is shared with a device might raise the question what else is shared to them, and this can now be found out without remembering the name and scrolling through a possibly long device list to do two clicks there.

My mistake was that I jumped straight into details, simply assuming that the controller changes were necessary even though they ended up being superfluous. As for the PR itself and its relevance to the issue, I also understood that it was about linking each device listed in the “Shared With” panel to its edit modal directly, which made perfect sense for me, as it does save time to view and edit device details this way instead of having to search for it in a potentially long list of devices separately.

Yeah, I would add that GitHub didn’t help there, as for me it showed all those lines as 100% changed because of the tabs that were used instead of spaces in the original commit.

It currently saves exactly one click anyways? Namely the action to unfold the related device-pane in the device-list. It would be more useful to go directly to the shared folders-settings of said device or the share settings of the folder. But going to the general tab of the device dialog? That’s fairly odd.

Sure, that device-list may in theory be quite extensive so some searching would be involved. But in practise the 95-th percentiles have 10 devices and 11 folders configured respectively. The ones with 669 devices or 1.7k folder configured most likely can’t really optimally use the GUI anyways for a plentiful of reasons.

Yes

Yes what? :wink:

  1. Do we have a consensus that opening the device editor is the right thing to do when clicking on a remote device name?

  2. Do we have consensus that the Sharing tab should be pre-selected in that case?

My personal stance is YES on 1 / NO on 2. It cannot be deduced that clicking on a device name there is always done to see the Sharing settings. If we leave it unspecified, the last used tab is opened, which is always a safe bet as it works for repetitive tasks and requires only one extra click the first time. My main motivation here is to save manual searching through the device list (which is long enough but not close to 50 in my usual cases).

It’s not a random place where we click on a device name, it’s in a specific folder regarding the ‘shared with’ settings. With that context in mind, it makes sense that the user would want to change the share-settings?

In my opinion this leads to:

  1. No
  2. No

Namely just a shortcut to that folder’s shared-with settings suffices.

Say we have 10 folders, all being shared with device-A. Why do we need 10 shortcuts to device-A? It makes absolutely zero sense.