Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui: Use discovered IDs from cache when adding a new remote device #8382

Merged
merged 3 commits into from Jul 22, 2022

Conversation

acolomb
Copy link
Member

@acolomb acolomb commented Jun 11, 2022

Purpose

The GUI already does refreshDiscoveryCache() when it comes online and on every 10 second refresh cycle. Querying the same information freshly in addDevice() seems unnecessary and adds some more round-trip delay on a possibly slow network link. Instead use the IDs from the existing discoveryCache property to populate the suggested ID list.

Increase maximum suggested device ID count to 100, only for the auto-completion list. It is hidden by default and filtered
by the browser, so we can offer more discovered device IDs without causing much disturbance. The list of suggested "nearby" devices is still limited to the first five.

Testing

Manual GUI testing with locally discovered, not yet added devices.

The GUI already does refreshDiscoveryCache() when it comes online and
on every 10 second refresh cycle.  Querying the same information
freshly in addDevice() seems unnecessary and adds some more round-trip
delay on a possibly slow network link.  Instead use the IDs from the
existing discoveryCache property to populate the suggested ID list.
For the auto-completion list, which is hidden by default and filtered
by the browser, we can offer more discovered device IDs without
causing much confusion.  The list of suggested "nearby" devices is
still limited to the first five.
The old name "discovery" was pretty ambiguous..
@acolomb acolomb added the ui Issues related to the graphical user interface label Jun 11, 2022
@acolomb
Copy link
Member Author

acolomb commented Jul 21, 2022

Ping. Review please?

Copy link
Member

@calmh calmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@calmh calmh merged commit 9c2428c into syncthing:main Jul 22, 2022
@acolomb acolomb deleted the gui-discovery-from-cache-when-adding branch July 22, 2022 09:30
calmh added a commit that referenced this pull request Jul 22, 2022
* main:
  lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes #8296) (#8427)
  gui: Move filesystem watcher explanation from tooltip to help block (#8432)
  gui: Use discovered IDs from cache when adding a new remote device (#8382)
  build: Update goleveldb (#8440)
calmh added a commit to calmh/syncthing that referenced this pull request Jul 26, 2022
* main:
  lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427)
  gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432)
  gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382)
  build: Update goleveldb (syncthing#8440)
@calmh calmh added this to the v1.21.0 milestone Jul 26, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Jul 28, 2022
* main:
  cmd/syncthing, lib/config: Remove restartOnWakeup option & functionality (fixes syncthing#8448) (syncthing#8449)
  gui: Remove blank meta tags (syncthing#8362)
  gui: Add device sync status (fixes syncthing#7981) (syncthing#8401)
  gui: Fix detailed staggered versioning information in folder info (ref syncthing#8348) (syncthing#8433)
  all: Support syncing ownership (fixes syncthing#1329) (syncthing#8434)
  gui, man, authors: Update docs, translations, and contributors
  lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427)
  gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432)
  gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382)
  build: Update goleveldb (syncthing#8440)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378)
  gui, man, authors: Update docs, translations, and contributors
  lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422)
  gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Jul 22, 2023
@syncthing syncthing locked and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion ui Issues related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants