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

Tab close button messed up #442

Closed
3 tasks done
tessus opened this issue Nov 18, 2020 · 7 comments · Fixed by #443
Closed
3 tasks done

Tab close button messed up #442

tessus opened this issue Nov 18, 2020 · 7 comments · Fixed by #443
Labels
App Issue affects the standalone application bug
Milestone

Comments

@tessus
Copy link
Contributor

tessus commented Nov 18, 2020

Prerequisites

  • Tried the most recent nightly build
  • Checked if your issue is already reported.
  • Answered all the questions in this template (Or provide a working crystal ball).

What happened?

The close button is messed up.

It seems that the src/app/libs/managesieve.ui/tabs/editor.tab.tpl has an encoding issue:

0.5.2:

<li class="nav-item">
  <a class="nav-link" id="profile-tab" data-toggle="tab" role="tab">
    <span class="siv-tab-name">NAME</span>
    <span class="close" style="padding-left: 5px; font-size: 1.2rem; float:unset">×</span>
  </a>
</li>

0.5.3

<li class="nav-item">
  <a class="nav-link" id="profile-tab" data-toggle="tab" role="tab">
    <span class="siv-tab-name">NAME</span>
    <span class="tab-close" style="padding-left: 5px">🗙</span>
  </a>
</li>
  1. open app
  2. connect to server
  3. open sieve script
  4. see close button:

image

What did you expect to happen?

It should look like in version 0.5.2

Screenshots

see above

Which Version

git master db3aef3

@thsmi
Copy link
Owner

thsmi commented Nov 19, 2020

It seem, as if the file is loaded on macOS as ASCII instead of UTF-8.

I can't reproduce this on Windows or Linux:
image

I also downloaded the mac image, unpacked it and checked the file in an hex viewer:

image

The code point looks as it should be:
https://www.compart.com/en/unicode/U+1F5D9

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2020

Hmm, I checked. The file is actually a UTF8 encoded file:

$ file src/app/libs/managesieve.ui/tabs/editor.tab.tpl
src/app/libs/managesieve.ui/tabs/editor.tab.tpl: UTF-8 Unicode text

Even the hexcode is the same. This makes no sense:

image

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2020

Dumb question: what happens on your Windows/Linux box when you run my PR #443? Does it still show the correct icon or is it now messed up on those systems?

@thsmi
Copy link
Owner

thsmi commented Nov 23, 2020

You pull request renders correctly on windows and linux.

There is one tiny difference between the two screenshots. If you check the UTF-8 rendering in your screenshot you'll see that a place holder is displayed. While in mine screenshot it is rendered correctly. So this looks to me, as if the code point is missing in the default macOS font.

Can you check if the code point ✕ ( U+2715 MULTIPLICATION X) is rendered on macOS correctly? It is a bit larger than the rather tiny × ( U+00D7 MULTIPLICATION SIGN) code point from your pull request.

@thsmi thsmi added bug App Issue affects the standalone application labels Nov 23, 2020
@thsmi thsmi added this to the 0.6 milestone Nov 23, 2020
@tessus
Copy link
Contributor Author

tessus commented Nov 23, 2020

Ok, both render perfectly:

U+00D7
U+2715

What does not render at all is U+1F5D9 (which you mentioned #442 (comment)). And that's the one that is in the file src/app/libs/managesieve.ui/tabs/editor.tab.tpl in 0.5.3.
In 0.5.2 it was U+00D7

So this looks to me, as if the code point is missing in the default macOS font.

Yes, this seems to be the case. So the easiest fix would be to use U+00D7 again - as in 0.5.2. (Which is what my PR does.)

@thsmi
Copy link
Owner

thsmi commented Nov 24, 2020

Thank you for the feedback.

I think I'll go with the U+2715. The symbol is larger, and this was the original intention for doing this change..

@thsmi thsmi changed the title tab close button messed up Tab close button messed up Nov 24, 2020
@thsmi
Copy link
Owner

thsmi commented Nov 24, 2020

The change set is now merged

@thsmi thsmi modified the milestones: 0.6, 0.6.0 Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App Issue affects the standalone application bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants