Tab component abstraction

[chengr4]

For #1817

[chengr4]

Tab UI Added

[Kobzol]

Hi, thanks for the PR. I think that we can do this in parts, i.e. in the first PR, just introduce tabs, and add more stuff in later PRs.

I'm not sure how should the tabs work though. Compile-time vs benchmarks optimized for size are completely separate "axes", and it doesn't really make sense to treat them as being on the same level.

I think that we could do sort of two level grouping. On the top level, there could be two tabs (compile-time vs runtime benchmarks), and then inside each tab, there could be several subsets of the data, and some of them could be collapsed. Something like this:

Again, I would start by just adding the tabs, maybe with first just adding a single tab for compile-time benchmarks, and not modifying anything else. It would also be nice to reuse the tab component from the compare page, if it could be done without complicating its code too much.

[chengr4]

How about also add the runtime tab, and indicate "to be continued" explicitly 🤔?

[Kobzol]

I don't think there's a lot of value in that, but if you want, feel free to add it. The point that I was trying to make is to do small PRs that are easy to review, while still keeping the code working in the meantime :)

[chengr4]

@Kobzol
I basically just abstracted Tab component.
I will pause here until you agree to proceed. When you have time, please chech it out. Thank you.

I personally find abstracting Tabs to be quite challenging. However, if you believe it's necessary, please instruct me. Alternatively, my next step could be to add tabs on the graph page.

[Kobzol]

I agree that it could be unwieldy to generalize the tab component, but I like the current state of the PR - CSS styling and switching of tabs can be kept in the Tab component, and just the inner content is replaced with the slot. That seems straightforward enough. And since we will probably use tabs on more places, I think that it's nice to encapsulate it like this, to avoid repeating the styling and selection logic of the tabs.

So 👍 from me.

[chengr4]

And since we will probably use tabs on more places, I think that it's nice to encapsulate it like this, to avoid repeating the styling and selection logic of the tabs.

Challenge accepted 🫠🫠

[chengr4]

@Kobzol
I feel before I keep going, I should share the current state with you.

Basically, the Tabs was abstracted. I created an array for creating TabComponent and passed it into Tabs.
Moreover, all calculation and SummaryTable were lifted to compare/page.vue. I wish I am on the right path.

I was thinking a better way to handle those calculation rather than put them all in the compare/page.vue. I also found out we got another SummaryTable. Maybe it is another refactoring issue.

If you have any opinion please let me know 🙏.

[Kobzol]

Oh, you also made the tab switcher generic 😅 I liked that you made the single tab to be a separate component, to avoid duplicating its DOM and CSS. I pretty much wanted to stay at that, and merge the PR once it was cleaned up.

Having the "tab switcher" generic is even better, but it should be really a generic component. Currently, you hardcode the Tab type in the tab switcher, and also compileTimeSummary, runtimeSummary etc. These types shouldn't be known to a generic tab switcher component. The tab switcher should just receive N tabs (Vue components), handle click events on each tab, and then somehow mark the selected tab. I think that a reasonable API for that would be to use something like React render props (maybe there's some equivalent in Vue?), so that the switcher uses a function to render each tab, and passes the function an index that states which tab is selected. Something like this:

<TabSwitcher tabCount="3" :renderTab="(index: number) => renderTabWithIndex(index)" />

The above interface example uses integer indices, to make the tab switcher generic. Maybe there is some TypeScript type-level magic that would allow passing in an iterable type (generic enum?) that would allow selecting the individual tabs using a proper type, rather than just indices, but it could be quite ugly.

If you want to, you can try to see if you can get something done with TypeScript so that we can pass arbitrary types to the tab switcher, but if it doesn't work out, I think that just using indices would be good enough.

[chengr4]

Lol
I misunderstood you earlier. Actually, I also preferred the state before I abstracted TabSwitcher.

For challenging of generalizing TabSwitcher, we can leave it for next time.
I will revert commits and clean up.

But really thank you for the advice and code review

[chengr4]

@Kobzol
I reverted commits, please take a look to see if there are any other modifications needed,
when you are available.

If everything's fine, we can merge this PR. I'll open a new one to create tabs for the graph page.

[Kobzol]

One final nit, otherwise LGTM.

[Kobzol]

Thanks! :)