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

Tabs #1120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Tabs #1120

wants to merge 1 commit into from

Conversation

sfsam
Copy link
Contributor

@sfsam sfsam commented Nov 18, 2020

This PR introduces a new tabline UI to replace PSMTabControl.

Rather than trying to mimic the ever-changing style of macOS, these tabs look like tabs. The rollover animations fade in & out instead of simply toggling on & off and there are some animations for closing and adding tabs as well.

Unlike PSMTabBarControl, there is no overflow menu. Instead, tabs scroll horizontally when there are too many to fit in the window at once.

There are properties for colors that match the Tabline* highlight group so that the tabs can match the user's colorscheme. However, I couldn't figure out how to get colorscheme colors programatically so for now colors are algorithmically generated based on the default foreground and background colors set in MMVimView.m.

The implementation uses ARC. It was build on macOS Mojave with Xcode 10.

I realize no one asked for this. I made it to scratch an itch and learn a little about MacVim. I'm putting it here in case others find it interesting or useful. This seems to work for me so far, but I'm no Vim or MacVim expert so it is possible I've majorly missed the mark!

CleanShot 2020-11-17 at 21 10 04@2x

@ychin
Copy link
Member

ychin commented Nov 18, 2020

Hi, I haven't taken a close look yet, but is there a reason you are implementing a new tab control instead of using an external third party one like those used by iTerm?

@sfsam
Copy link
Contributor Author

sfsam commented Nov 19, 2020

A couple reasons: ease and functionality.

Ease: The variant of PSMTabBarControl in iTerm2 is pretty big: 33 files (16 objc, 17 header) and 6,200 lines of code. My implementation is 6 files (3 objc, 3 header) and 660 lines of code. It was a lot easier and less work starting from scratch than understanding and customizing a large codebase just for tabs with the specific functionality I was targeting. It is easier to debug and maintain a significantly smaller base.

Functionality: I was targeting specific functionality. I wanted to eliminate the overflow menu that MacVim and iTerm2 have when there are too many tabs for the window. With scrolling tabs, you always see the selected tab. You can always drag to reorder any tab, not just those that aren't hidden in a menu. It means the + tab button is always available and never overloaded with the >> menu button.

Since MacVim is Vim, I wanted to design tabs that would work with Vim's Tabline highlight group so users could customize their tabline in MacVim just as they do in Vim. So the parts of my tabs correspond to Tabline, TablineSel, and TablineFill. The MMTabline class has properties for these. I couldn't figure out how to access these colors from MacVim, so for now I generate these properties algorithmically based on MMVimView's default foreground and background colors. It seems to work for the ones I tested:

Default:
CleanShot 2020-11-18 at 20 23 29@2x

Solarized Light
CleanShot 2020-11-18 at 20 23 37@2x

Solarized Dark
CleanShot 2020-11-18 at 20 23 52@2x

Monokai
CleanShot 2020-11-18 at 20 23 44@2x

For closing multiple tabs, I wanted to mimic Chrome's behavior where tabs conveniently slide under your mouse and then resize when you leave the tabline. I don't think I could do this with PSMTabBarControl:

CleanShot 2020-11-18 at 21 15 40

Overall, I was looking for specific functionality and it was a lot easier to achieve what I was after by starting from scratch. At around 660 lines of code, I don't think it resulted in a lot of new code. FWIW, I believe the executable is also around 300K smaller.

@ychin
Copy link
Member

ychin commented Nov 21, 2020

I see. Ok it will probably take me some time to review this and there's a backlog of other PRs and issues to fix for Big Sur so this will likely not make it in the next release. I'll get to it though!

@sfsam sfsam force-pushed the tabs branch 2 times, most recently from d247599 to da2a243 Compare December 20, 2020 19:05
@ychin
Copy link
Member

ychin commented Dec 21, 2020

I was playing around with the new tabs and they look pretty good! I'll dig deeper to give feedbacks but I think we do want this. Meanwhile, you are still working on it right? Let me know when it's no longer WIP.

Couple comments so far:

  1. If you have too many tabs you will need to scroll since the implementation is avoiding making the tabs too small. However, I couldn't find a way to scroll without a trackpad. I think we should think about how to allow the user to select off-screen tabs. We should at least make sure scrollwheel would scroll the tabs. For users without tabs, in the native tabs / Safari implementation, they would bundle the tabs to one side so you could click on it to pan left without needing a trackpad or scrollwheel, as follows:
    image
    Alternatively, in iTerm2 implementation uses a dropdown:
    image
    Note that we can't do drag-to-pan easily because dragging is already used for re-ordering tabs which I think makes sense. Not saying we should do the same thing, but we should provide some way to select / scroll to those tabs without a requirement of a scrolling device.

  2. I find the top part slightly distracting as there is some empty space there. Is there mostly a stylistic choice? Have you tried it out to not have those empty space and how it looks? Actually from your screenshot it does seem like there is no space on top. I wonder if we have different configurations / OS.

  3. I like that it's using the colorscheme for coloring. I'm debating if we need some ways to allow customization of the tab bar or lay the groundwork for that. No concrete thoughts on this yet.

@sfsam
Copy link
Contributor Author

sfsam commented Dec 23, 2020

Thank you for taking the time to look at this. I know you're busy and no one asked for tabs - I was just scratching my own itch.

  1. You are right. I've been using a laptop for so long, I sort of forgot about mice! I will think about this and get back to you. One of the points of this implementation was to not have an overflow dropdown menu. I think those are sort of like hamburger menus - a place to put the UI you couldn't solve properly. Right now I don't even own a mouse so I might have to get one...
  2. I started without any space above the tabs, but added it after I switched to Big Sur since the titlebar now renders in a color similar to the background. Having the tab background color (TablineFill) above the tabs adds contrast and visually separates the selected tab from the titlebar. It's similar to how Chrome and Sublime Text render tabs. I think it looks better and is easier to parse visually. It's easy to change, but I'd suggest getting functionality nailed down (like mouse UI!) before aesthetics since one can go down a rabbit hole iterating tweaks.
  3. My aim was to use the existing Vim Tabline, TablineSel, and TablineFill highlight groups. People have already spent a lot of time customizing their themes. Furthermore, terminal Vim does it this way so I think this would be expected behavior for a GUI. But I didn't know how to access these values from code (help!), thus my colorscheme hack.

@ychin
Copy link
Member

ychin commented Dec 23, 2020

  1. I don't think you need a mouse. Just use a trackpad but try to navigate the tabs without the scroll gesture :). Some suggestions:

    • Reserve some space on the left/right edges so if you click on them you will scroll the tabs instead (if there are more tabs on that side). This is kind of how macOS handles tabs anyway, except it also bunches up the tabs to make it clear in the UI there are more tabs to access.
    • Overflow dropdown menu: To be fair I think the main issue with the menu was that it's the only way to access the overflow tabs, since the old tabs could not be scrolled. Now that they could be scrolled it would more be an optional use (maybe it's optional to show) in case someone is using a mouse without a scrollwheel.
    • Support right-drag or middle-drag to pan. I don't love this because right-click should be for the menu, and middle click usually means close tab on a browser.

    FWIW I think the behavior works pretty well right now. We should just give something to make it usable for people who don't have the ability to scroll. (Well, technically, they could use the :tab commands, but that would defeat the pt of the UI)

  2. Huh, let me take a look at Big Sur. I was using Catalina when I looked at your change. Yeah the behavior is more important to get right first. Also, if you are using Big Sur, make sure it works with/without toolbars (set guioptions+=T).

  3. Let me take a look at how you did it again.

@eirnym
Copy link
Contributor

eirnym commented Dec 23, 2020

  1. I'd add an option for this. I think macOS defaults will be a good storage for this

@sfsam
Copy link
Contributor Author

sfsam commented Dec 23, 2020

  1. I'll try some things out.
  2. I'll check the T guioption.
  3. Take a look at MMVimView.m Inside -setDefaultColorsBackground:foreground: I call my method -setTablineColorsBasedOnBackground:foreground. There, I infer colors for the tab parts based on the foreground and background parameters. But I'd prefer to just set them according to the user's colorscheme.

@ychin ychin added the UI Issues related to UI elements, tabs, scrollbars, window resizing, etc. label Dec 26, 2020
@ychin
Copy link
Member

ychin commented Dec 26, 2020

  1. Ah right yeah that's kind of hacky. If you want to pass the highlight colors for Tabline/etc up you would have to add some hooks. Vim lives in a different process from MacVim, and the Vim-side code gui_macvim.m and MMBackEnd.m sends messages to MacVim. For setting the normal colors, gui_macvim.m's gui_mch_new_colors() calls MMBackEnd's setDefaultColorsBackground, which sends an IPC message over to MacVim of type SetDefaultColorsMsgID. You could either add to that message or make a new one.

If you want to see to see how gui_mch_new_colors() gets called to begin with and where it got the colors from, if you look around, it's called by set_normal_colors() in highlight.c, and you can look at set_groupd_colors to see how it does the lookup. You could just look up the colors similarly.

That said, I was looking at builtin and other popular colorschemes, and I'm not 100% sure their tabline colors would look good in the GUI. I recommend manually plugging in the color for debugging first to see if it even looks good. It sounds good in theory but those colors were designed with the terminal-based tablines in mind and may not look as good as you hope.

@sfsam
Copy link
Contributor Author

sfsam commented Dec 27, 2020

  1. I've added some tab scroll buttons to the left of the tabline. There is a user default (MMShowTabScrollButtonsKey) that can be used to hide them since they aren't needed for trackpad users. They are shown by default.

@sfsam
Copy link
Contributor Author

sfsam commented Jan 2, 2021

  1. I looked at highlight.c and gui_macvim.c but wasn't able to reliably get Tabline colors. I'll keep looking at the Vim source, but right now I'm stuck. Instead, I found a formula from the W3C that calculates perceptual brightness and used it to improve the algorithmic color selection based on the default foreground and background colors. I agree that some of the tabline colors in the color schemes might not look great whereas I think the algorithmic colors work pretty well now. I'm thinking in the future, it would be nice if the tabs provided a good default experience (algorithmic colors), but afforded motivated users the option to override the default experience. I imagine a check box (NSUserDefault BOOL property) where a user could indicate they opted to use their own color scheme's Tabline highlight group colors instead of the default algorithmic colors. I've also reverted the extra space above the tabs.

@ychin
Copy link
Member

ychin commented Jan 3, 2021

  1. OK that sounds good. Yeah if some of the tabline colors don't look good let's just stick with the algorithmic one for now. I could wire in the Vim binding stuff if you couldn't quite figure it out (there are quite a few bits of indirections there) and we could add that later.

@ychin
Copy link
Member

ychin commented Jan 3, 2021

  1. I was playing with your scroll buttons, and they seem to work well! Thanks for adding the option. Did you try only making them show up if we are overflowing? Also, I was debating to myself whether I think it belong to the left or right side.

@sfsam
Copy link
Contributor Author

sfsam commented Jan 3, 2021

  1. The buttons automatically enable/disable if there is scrollable content. I did try having them show/hide, but it makes the UI seem glitchy. I think for interfaces like this, having them enable/disable is most natural. I initially had the scroll buttons on the right with the "+" button but it looked busy. I also did end up buying a mouse for testing, but I got stuck on getting the scroll wheel on the mouse to scroll the tabs horizontally. I feel like there is something obvious I'm missing, but I don't have any idea what it might be.

@sfsam
Copy link
Contributor Author

sfsam commented Jan 3, 2021

  1. Ok, I just learned that to scroll horizontally with a scroll wheel, you hold down the shift key. So it actually does work, I just didn't know how to use a mouse. :)

@ychin
Copy link
Member

ychin commented Jan 4, 2021

Mouse wheel scrolling: Yeah but most users are not going to know to do that (holding shift). It's just unnecessarily difficult. So here's my suggestion: On trackpad / magic mouse, use horizontal scrolling just like now because it's more intuitive. On a regular mouse, use vertical scrolling because most mouse only have that. The best way I know to tell whether it's a regular mouse or a trackpad (or Magic Mouse) is by using hasPreciseScrollingDeltas, so I think you can use that to decide whether to use X or Y axis scrolling (Y-axis scrolling is basically traditional scrolling, whereas X / horizontal scrolling isn't standard).

Also, thanks for going the extra mile to test it with a mouse!

For reference, this behavior also seems to be how macOS native tabs work. Actually, to be more accurate, seems like native tab doesn't care whether you shift-scroll or scroll, so maybe they just allow either X or Y axis for scrolling, so I think that option seems ok as well.

@ychin
Copy link
Member

ychin commented Jan 4, 2021

Left/right buttons: I see. Hmm, I can kind of see that. I'm still wonder what "glitchy" means in this case. Are the tabs jumping around? I was imagining it would just push the tabs to the right, so I'm curious. Is the code easy to enable in the code right now? I may play around with it.

Otherwise, I think maybe we could make it disabled by default. I definitely think we should keep the feature but most people may not need it, so we just want to have the option for those who do.

@sfsam
Copy link
Contributor Author

sfsam commented Jan 4, 2021

Mouse wheel scrolling: How do I get the scrolling deltas? I had tried overriding -scrollWheel: but it was never called. So that's where I was stuck and why I felt I was missing something obvious.

Left/right: By glitchy I meant having the buttons continually appear and disappear as you scrolled back and forth. It's more jarring than having them dim. I think it's also a bit of an anti-pattern to have UI controls just disappear. For example, browser back/forward buttons don't disappear when there is no history, they just dim.

For sure, you can certainly play around with it!

I noticed once I plugged in my mouse that scroll bars appeared on all my windows. Unplugging the mouse made them automatically disappear. I wonder how the presence of the mouse was detected and if the buttons could automatically toggle based on that.

@ychin
Copy link
Member

ychin commented Jan 11, 2021

Hmm, I think it has to do with how the NSWindow is configured. I thought you are overriding scrollWheel: event, but it may be because you just make it a scroll view and macOS is handling for you automatically. I need to take a look at how you are organizing the windows but I imagine you could override scrollWheel: at the correct place, and maybe just send deltaY to deltaX and that should work.

@sfsam
Copy link
Contributor Author

sfsam commented Jan 13, 2021

When I overrode -scrollWheel: in MMTabline.m, it was never called. So then I tried creating my own NSScrollView subclass and overriding it there instead. In that case, my -scrollWheel: override was called, but trackpad scrolling no longer worked. Someone actually mentions this behavior on StackOverflow (https://stackoverflow.com/q/43854025/111418) but the question has no responses. I know the underlying scrolling machinery was heavily modified in macOS 10.9 for responsive scrolling and I wonder if that has something to do with it?
https://developer.apple.com/library/archive/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_9Scrolling

@sfsam
Copy link
Contributor Author

sfsam commented Jan 18, 2021

Mouse wheel scrolling: I think I've got it working. Rather than overriding -scrollWheel:, I'm using something called an event monitor to watch for scroll wheel events. I take the original scroll wheel event and create a new one based on it where the delta X value is the original event's delta Y. Scrolling with the SHIFT key also still works.

@ychin
Copy link
Member

ychin commented Mar 8, 2021

Hi @sfsam I haven't checked up on the progress here much. Is this ready to merge and should I take a closer look? Was just curious since the title still says "WIP"

@sfsam sfsam changed the title WIP: Tabs Tabs Mar 9, 2021
@sfsam
Copy link
Contributor Author

sfsam commented Mar 9, 2021

Hi @ychin. Yes it's ready for you to take a closer look.

@tucnak
Copy link

tucnak commented Aug 27, 2022

Yes, I've tried this PR, but my main issue with it is that it doesn't follow the transparent title bar logic. And this is something I genuinely love about MacVIm. I'm sure @sfsam did a great job, and if anything this looks better than what we have now, but just falls short of perfection due to the inability of this design to adapt whenever the title bar is transparent. This is what this PR looks like on my machine & my config:

image

Note: the left and right buttons in the left do nothing.

@ychin
Copy link
Member

ychin commented Aug 27, 2022

What does not following transparent title bar logic mean? You want the gray area to the right to be just the background color? Note that the above proposal of "tabs in titlebar" is something else (e.g. iTerm calls them "minimal style")

@tucnak
Copy link

tucnak commented Aug 27, 2022

When I say transparent title bar logic, I have in mind the "Titlebar appearance -> Transparent" setting, which displays the title bar within the canvas of the editor and embeds into hi Normal or whatever. (And in iTerm they have this too, they call it "minimal," correct.)

image

You want the gray area to the right to be just the background color?

I say simply move the existing tab bar in the proposal to the title bar, and yes, make its background transparent, not gray.

@ychin
Copy link
Member

ychin commented Aug 27, 2022

When I say transparent title bar logic, I have in mind the "Titlebar appearance -> Transparent" setting, which displays the title bar within the canvas of the editor and embeds into hi Normal or whatever. (And in iTerm they have this too, they call it "minimal," correct.)

Sure, that's not really what transparent titlebars are for. The purpose of transparent titlebars is to make it use the background color, instead of drawing the macOS titlebar. It would be a new feature request to embed the tab bars within the titlebars, and it would be called something else (e.g. "minimal", "compact", etc)

That aside I don't think embedding the Vim editing area itself within the titlebar makes sense, because you won't be able to click on things (you either can't click on those area of the editing area, or the title bar). You can already set MacVim titlebar to "hidden", with the obvious drawback that you can drag it around.

I say simply move the existing tab bar in the proposal to the title bar, and yes, make its background transparent, not gray.

That could be a future option but definitely not the default. It breaks what the default assumptions of how tabs should work in most applications. FWIW Safari during Monterey beta tried doing that and it was so unpopular that they reverted back to the regular tabs, with the "compact" mode as an option.

Edit: this style of tabs also breaks tool bars, and I think you are getting less space for your tabs (if you have a lot of tabs), so I'm personally not too big a fan of them as I think they only work well if you have a couple tabs. I'm not opposed to adding them in the future but just want to point out that features like this have a lot of knock-on effects (e.g. the tool bar issue that I just mentioned)

@eirnym
Copy link
Contributor

eirnym commented Mar 10, 2023

@sfsam thank you to Keep working on that feature!

@ychin
Copy link
Member

ychin commented Mar 10, 2023

I'm sorry that this has fallen by the wayside for me. I need to get back to taking a look and merging this. Since you seem to have kept up merging from upstream is this ready to go @sfsam ?

@sfsam
Copy link
Contributor Author

sfsam commented Mar 11, 2023

Hi. Yes @ychin it is ready for you to take a look. I've just been sporadically keeping it up to date with the current state of MacVim. Since this is a big change, I'm nervous about it and hope it can be tested by folks who use GUI tabs regularly, both for functionality as well as for bugs. On a personal note, I have some "family/life things" that I need to take care of. They are starting to swallow me up and consequently most of my attention and energy is focused there.

@ychin
Copy link
Member

ychin commented Mar 11, 2023

On a personal note, I have some "family/life things" that I need to take care of. They are starting to swallow me up and consequently most of my attention and energy is focused there

Oh sure that's fine. Given that this pr has sat unreviewed for so long I do appreciate you having merged and kept it up to date so far.

If you do not mind me fixing up the PR here and there I may do so just so we can merge it.

@sfsam
Copy link
Contributor Author

sfsam commented Mar 11, 2023

Thanks @ychin. Sounds good.

@huyz
Copy link

huyz commented Oct 7, 2024

@ychin ping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Issues related to UI elements, tabs, scrollbars, window resizing, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants