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

Cosmetic improvements to menu text. #199

Closed
GWHAYWOOD opened this issue Oct 6, 2021 · 18 comments · Fixed by #200
Closed

Cosmetic improvements to menu text. #199

GWHAYWOOD opened this issue Oct 6, 2021 · 18 comments · Fixed by #200

Comments

@GWHAYWOOD
Copy link

  1. In Options -> "Tab Bar" -> "Tab Bar Fullscreen Behaviour"
    The options mention (Spitter visible) or (Spitter hidden).
    I do not know what a Spitter is, perhaps the word should be "Splitter" but I still would not know what it is. If it means the little 'plus' symbol which expands a collapsed group then perhaps it could be called the "expand button" or something like that. Also, if it does mean the little 'plus' symbol, the setting the option to "hidden" seems to have no effect.

  2. In Options -> Grouping -> "Positioning Tabs"
    One option for "Ungrouped new tabs should be positioned" is "at far right of the tab bar".
    This makes no sense if the tab bar is at the left or right edges of the screen (and is thus vertical).
    Perhaps a better wording might be "after all other tabs".

@PikachuEXE
Copy link
Member

(1) Would it be better to make preference panel to display description like the following?
image

(2)
How about "as last tab"? (from Vivaldi)

@GWHAYWOOD
Copy link
Author

Looks fine to me. The description of the splitter helps a lot - at least it's helped me - because now I know what to look for I can see that the option does not work. The splitter remains visible when the option says it is hidden.

(I guess you know that it still says "Spitter" instead of "Splitter" in the drop-down.)

@PikachuEXE
Copy link
Member

Spitter...
Typo!

The splitter remains visible when the option says it is hidden.

That another bug I need to fix...

@PikachuEXE
Copy link
Member

Updated
image
image

@PikachuEXE
Copy link
Member

Let me know if there are any more changes
If not I will merge #200 next week

@GWHAYWOOD
Copy link
Author

I can't see any of the proposed changes, when I try I just get a little spinning circular thing that goes round and round apparently forever. Now that I've become more familiar with the UI, I haven't lately spent much time browsing the menus so I haven't been so sensitive to little quirks. If you've dealt with what I've mentioned so far then I think you've fixed the most obvious.

Did you get to the one I mentioned about sometimes seeing

RightClick->Tab Kit->"Group Tabs From Here To..."

instead of

RightClick->Tab Kit->"Group Tabs From Here To Current Tab"

or whatever it is, where I'm always praying that the ellipsis is an abbreviation of "Current Tab"?

When the version with the ellipsis appears it always seems to be pointless abbreviating the text. There always seems to be plenty of room for the text to be displayed in full, and some other items in the same menu are much longer. It's just scary to I that think I might not be about to do what I think I'm about to do!

@PikachuEXE
Copy link
Member

I just take a look at the translation and it seems using Group Tabs From Here To Current
image
image

So what you see might caused by something else (another bug?)
But I am not sure how to reproduce yours.
Maybe upload a screenshot as a start.

@GWHAYWOOD
Copy link
Author

http://www.jubileegroup.co.uk/misc/TK2_group_tabs_from_here.png

Sorry, still don't know of a way with this abysmal UI to attach a file. Drag/drop, select/paste have no effect.

@PikachuEXE
Copy link
Member

WEIRD

You can see the source code referenced below which does not use ... at all
https://github.com/tabkit/tabkit2/blob/master/src/locale/en-US/overlay.dtd#L70-L82

And I don't have a Linux version to reproduce

I can only show you windows version
image

@GWHAYWOOD
Copy link
Author

Are you sure that's the right bit of code? Two lines below in your screenshot it says "Mark As Unread" but in the code it says "Mark As Read". Then again maybe something is being 'clever' or 'helpful' with the rendering. I hate software when it does that. Maybe it would render more like the 'Close Tabs...' line if the text were to be made just a character or two shorter. Also I wonder what would happen if it said

"Group Tabs From Current To Here"

At least it's always 'Here' to somewhere so you wouldn't need to guess. :)

If you look at the 'Close Group' line, the list of shortcut keys for that command (G; ALT+SHIFT+W;...) is quite long, and in horizontal extent definitely overlaps with the last characters of 'Close Tabs From Here To Current'. I wonder if something in the 'helpful' software is getting the line length calculations wrong for e.g. some fonts, or whatever it uses to calculate how much room it has to place the text. Whatever it is, it's clearly doing it very badly. It must be in whatever generates the menu, I don't see how it can be the browser.

@PikachuEXE
Copy link
Member

"Mark as Read": There is code to set it as right text
https://github.com/tabkit/tabkit2/blob/master/src/content/tabkit.ts#L3250-L3256

The menu definition is in
https://github.com/tabkit/tabkit2/blob/master/src/content/browserOverlay.xul#L196-L224
(highlighted "new tabs" to "close tabs" part)

AFAIK there is no custom style set by TabKit for menu items.
Those are defined via XUL provided element menuitem so they should be using the "default" style from Palemoon.

I can try shortening some text and create a test version to let you test whether the issue would be "fixed".
But I will only do that after #200 & #210 are merged.

@GWHAYWOOD
Copy link
Author

I guess we've beaten this one to death and it isn't worth any more of your time unless other people run into the same issue. I think I can be confident when the full text does not appear that it means what I think it means - and now that I'm using Session Manager it would be easier to recover if I did have an accident.

@PikachuEXE
Copy link
Member

Test version for all text changes so far
tabkit2_0.14.0-2021-11-10-114758.xpi.zip

@GWHAYWOOD
Copy link
Author

Thank you. Installed 11:30 UTC 2021-11-10.

When I restarted the browser, all my tabs appeared to be present and correctly sorted, although a small number of tab groups had been broken up into smaller groups. Unfortunately I forgot to check the tab groupings before the restart so I cannot be absolutely sure that this happened at the browser restart but I think it was. The broken groups were all in a single window. This was the window in which I was working at the time of the restart, I do not know if that is significant or just a coincidence. Next time I restart I will try to make sure I know exactly how all the tab groups appear before the restart. Generally, because of the problems that restarts usually cause, I try not to restart the browser very often -- but it will be worth doing it if it provides any more useful information for you.

In all, this is a very big improvement on the performance as compared with my early experience.

Thanks once again.

@PikachuEXE
Copy link
Member

I am gonna merge #200 about text update unless you have more suggestion that can be implemented soon 😄
I plan to release a new version before Xmas

@GWHAYWOOD
Copy link
Author

I still see the weird wording replaced by '...' thing - most of the time I see that rather than the full wording. I haven't yet firgured out what makes it show me the full wording without the '...' replacement but when I do I'll either add a note here or create a new issue, whatever you prefer.

@PikachuEXE
Copy link
Member

For the '...' issue I think it's better to open a new issue as it might only happens in some environment (OS, OS version, what else...?)

@PikachuEXE
Copy link
Member

Let's continue the discussion for the '...' issue in #212

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants