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

Newly added "connection type" icon and text in device info is slightly misaligned #8592

Closed
tomasz1986 opened this issue Oct 7, 2022 · 6 comments
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Milestone

Comments

@tomasz1986
Copy link
Contributor

This has been added recently in fd0a622. The problem is that the icon doesn't come from Fork Awesome but from Bootstrap. Because of this, it's got different dimensions, which makes it narrower than the other icons that come from Fork Awesome.

image

I can think of two possible fixes for this:

a) Tweak the CSS to make the icon's width identical to Fork Awesome icons. Maybe adding some padding/margin horizontally could fix this?

or

b) Replace the Bootstrap icon in this single instance with the signal icon from Fork Awesome, which uses the same dimensions as all the other currently used icons.

@tomasz1986 tomasz1986 added bug A problem with current functionality, as opposed to missing functionality (enhancement) needs-triage New issues needed to be validated labels Oct 7, 2022
@acolomb
Copy link
Member

acolomb commented Oct 8, 2022

To me it seems like the margin left of the icon is smaller but the distance to the following text is the same. So maybe our CSS somehow sets additional padding / margin on the FA icons, which doesn't get applied to the Bootstrap one?

The question whether Fork Awesome is still needed for licensing reasons was raised previously. If not, it might be best switching back to Font Awesome and use their icons consistently. Does anybody know more about the licensing concerns?

@bt90
Copy link
Contributor

bt90 commented Oct 8, 2022

 Fork Awesome is still needed for licensing reasons

They have near identical licenses?

https://fontawesome.com/license/free vs https://forkaweso.me/Fork-Awesome/license/

The only difference is CC BY 3.0 vs CC BY 4.0

@tomasz1986
Copy link
Contributor Author

tomasz1986 commented Oct 8, 2022

I think the license that applies to our use case is actually

Fonts - SIL OFL 1.1 License
In the Font Awesome Free download, the SIL OFL license applies to all icons packaged as web and desktop font files.

as our icons are technically fonts. We could of course replace everything with SVG images instead (which is better for accessibility, but it would also require quite a lot of work).

@wweich
Copy link
Member

wweich commented Oct 8, 2022

I don't know, if it makes a difference in license, but using the JS Version of Font-Awesome doesn't require any Fonts (just 1 js-File needed, nothing else) and it will replace the <i> -Tags with the corresponding SVG-Code automatically.
Placing

<script>
    FontAwesomeConfig = {
        autoReplaceSvg: 'nest'
    };
</script>

before the Font-Awesome-Script, the <i>-Tags won't be replaced, but the SVG-Code will be placed inside them, so the CSS-Selectors still work and most of the custom styling could still work.

@imsodin
Copy link
Member

imsodin commented Oct 9, 2022

It's about source code: Font awesome is only available in compiled form, build tools arent' open source. So for a strict definition of open source, it isn't - users don't have a means to modify it at will (as they can't compile modifications). That's an issue for debian, not sure if also for fedora or f-droid. So not an issue for our own builds/release, but at least for a small fraction of users it would be (according to usage reporting ~3%). Personally as long as we don't have a strong reason to, I'd rather not move to font awesome (just my personal opinion).
Original ticket about fork/font awesome: #5236

@calmh
Copy link
Member

calmh commented Oct 9, 2022

Maybe bootstrap icons is the better set, if we were going to switch to anything. (If it's open source enough, I didn't investigate beyond seeing "open source" in the page title.)

tomasz1986 added a commit to tomasz1986/syncthing that referenced this issue Nov 4, 2022
The connection type icon comes from Bootstrap. As such, it does not
follow the same dimensions as the other GUI icons, which come from Fork
Awesome. Thus, add left and right margin to make its width roughly the
same as the other GUI icons, which fixes its alignment in relation to
text.

Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
@calmh calmh closed this as completed in fbdaa26 Nov 4, 2022
calmh added a commit to imsodin/syncthing that referenced this issue Nov 8, 2022
* main: (36 commits)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  lib/fs: Let xattr test avoid non-test attributes (fixes syncthing#8601) (syncthing#8628)
  build: Add GitHub actions build for Windows
  gui, man, authors: Update docs, translations, and contributors
  gui: Display folder and device count number (syncthing#8615)
  gui, man, authors: Update docs, translations, and contributors
  lib/model, lib/protocol: Fix file comparisons (fixes syncthing#8594) (syncthing#8603)
  lib/scanner: More sensible debug output (syncthing#8596)
  gui: Allow automatic device ID selection on WebKit browsers (ref syncthing#8544) (syncthing#8597)
  ...
@calmh calmh added this to the v1.22.2 milestone Nov 9, 2022
calmh added a commit to calmh/syncthing that referenced this issue Nov 22, 2022
* main: (23 commits)
  lib/fs: Optimize WindowsInvalidFilename (syncthing#8687)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing: Use main logger in generate subcommand (fixes syncthing#8682) (syncthing#8685)
  build: Update all dependencies (fixes syncthing#8679) (syncthing#8680)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Correctly set xattrs on temp files (fixes syncthing#8667) (syncthing#8670)
  gui: Automatically dismiss authentication reminder when in LDAP mode (fixes syncthing#8661) (syncthing#8663)
  lib/model: Correctly handle xattrs on directories (fixes syncthing#8657) (syncthing#8658)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  ...
@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 Nov 5, 2023
@syncthing syncthing locked and limited conversation to collaborators Nov 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Projects
None yet
Development

No branches or pull requests

7 participants