Skip to content

feat: Add external links to tabs #2243 - #2501

Merged
mturoci merged 6 commits into
h2oai:mainfrom
himaSH97:feat/tab-external-link
Sep 15, 2026
Merged

mturoci merged 6 commits into
h2oai:mainfrom
himaSH97:feat/tab-external-link

Conversation

@himaSH97

@himaSH97 himaSH97 commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

The PR fulfills these requirements: (check all the apply)

  • It's submitted to the main branch.
  • When resolving a specific issue, it's referenced in the PR's title (e.g. feat: Add a button #xxx, where "xxx" is the issue number).
  • When resolving a specific issue, the PR description includes Closes #xxx, where "xxx" is the issue number.
  • If changes were made to ui folder, unit tests (make test) still pass.
  • New/updated tests are included

@himaSH97
himaSH97 requested review from lo5 and mturoci as code owners August 20, 2026 06:05
@himaSH97
himaSH97 marked this pull request as draft August 20, 2026 06:05
@himaSH97

himaSH97 commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Fixes #2243

Summary

Adds an optional link property to ui.tab.

When link is set, clicking the tab navigates to the supplied external URL.
Tabs without link retain their current behavior and continue emitting backend
events.

Tests

image

@himaSH97
himaSH97 marked this pull request as ready for review August 20, 2026 06:09

@mturoci mturoci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @himaSH97! The PR looks good, just a few comments/questions.

Comment thread ui/src/tab.test.tsx Resolved
Comment thread ui/src/tab.test.tsx Resolved
Comment thread ui/src/tabs.test.tsx Resolved
Comment thread ui/src/tab.tsx Resolved
Comment thread website/widgets/content/tabs.md Resolved
@himaSH97
himaSH97 requested a review from mturoci August 22, 2026 05:26

@mturoci mturoci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM.

Found 1 minor styling bug for visited link state. Would be better if the visited link style would be the same as non visited (respecting theme color).

Screen.Recording.2026-09-01.at.4.44.25.PM.mov

@himaSH97

himaSH97 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Added a small fix to above styling mismatch. Let me know if this works.

I have added a prevent default for the mouseDown event. it breaks the forcus from the rendered a tag and use the default styling.

@himaSH97
himaSH97 requested a review from mturoci September 2, 2026 04:48
@mturoci

mturoci commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Isn't there a way to just override :visited style for the link? Would be more explicit about the actual intent. The JS solution may also potentially turn off other browser behavior we would desire. Better to just fix styling and not touch anything else.

@himaSH97

himaSH97 commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi @mturoci,
I tried adding a custom class with the existing css styling but this method gets overridden by pivot link styles

className: css(className, isSelected && classNames.linkIsSelected)

I found it is possible to do this with custom html attribute ('data-external-tab': true) and use that to render the styles which pivot won't override

'.ms-Pivot-link[data-external-tab]:hover, .ms-Pivot-link[data-external-tab]:hover .ms-Pivot-linkContent': {
            color: cssVar('$card')
   }, 

is this a good approach to use here?

@mturoci

mturoci commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Hey @himaSH97. See b958521 for the proper solution.

I assume your attempts just needed an !important rule. However, we do not want to scope the fix to just tab components, but potentially globally to all the places that framework may need to render links within a Pivot items. The commit above fixes them all uniformly.

I tested around and couldn't find any more issues. Please do the same and let me know in case you still find some bug. If not, I we are good to merge this.

@mturoci
mturoci merged commit e01e8c1 into h2oai:main Sep 15, 2026
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 this pull request may close these issues.

2 participants