Skip to content

Conversation

@sangxxh
Copy link
Contributor

@sangxxh sangxxh commented Apr 21, 2021

This PR fixes #121521

microsoft_vscode_121521_After

}
}));

const onMouseOver = domEvent(this.el, 'mouseover');
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend to instead follow the existing pattern for styling links on hover using CSS, for example here's how the search view does it:

const activeLink = theme.getColor(textLinkActiveForeground);
if (activeLink) {
collector.addRule(`.monaco-workbench .search-view .message a:hover,
.monaco-workbench .search-view .message a:active { color: ${activeLink}; }`);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented it but realized it doesn't really work because Link is used in a few different places:

  • src/vs/workbench/browser/parts/views
  • src/vs/workbench/contrib/welcome/gettingStarted/browser
  • src/vs/workbench/contrib/workspace/browser

So it doesn't look like there's a safe CSS selector that can cover all those cases and potential future consumers of Link.

What's your thought?

Copy link
Contributor

Choose a reason for hiding this comment

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

You might add a class to the link's a element and style based on that, but I'll leave further review to @joaomoreno as he owns this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, that sounds like the way to do it. I'll have a go while waiting for @joaomoreno's review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No luck with registerThemingParticipant yet, I'll wait for @joaomoreno 's review for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any thoughts @joaomoreno ?

@joaomoreno joaomoreno added this to the Backlog milestone Apr 26, 2021
@joaomoreno joaomoreno added the workbench-welcome Welcome page issues label Apr 26, 2021
Copy link

@glaukiol1 glaukiol1 left a comment

Choose a reason for hiding this comment

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

solid fix, thanks for contributing, also, please make a default color! thanks

@joaomoreno joaomoreno merged commit 9ef2514 into microsoft:main May 17, 2021
@joaomoreno
Copy link
Member

Thanks! 🍻 Ended up cleaning up quite a bit how we style links.

@joaomoreno joaomoreno modified the milestones: Backlog, May 2021 May 17, 2021
@sangxxh sangxxh deleted the issue/121521-tree-view-welcome-content-not-using-hover-link-color branch May 17, 2021 08:18
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

workbench-welcome Welcome page issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tree View welcome content doesn't use hover link color

4 participants