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

Duplication/Layering in simpleFindWidget #33264

Closed
alexdima opened this issue Aug 28, 2017 · 0 comments
Closed

Duplication/Layering in simpleFindWidget #33264

alexdima opened this issue Aug 28, 2017 · 0 comments
Assignees
Labels
debt Code quality issues
Milestone

Comments

@alexdima
Copy link
Member

I've noticed some very fishy code when I looked inside the /editor/find folder thanks to #32113

  • the /editor/ folder should not take a dependency on the workbench. We don't have a linter warning / pre-commit hook for it, but using the CSS class name .monaco-workbench in the /editor/ folder indicates that something is wrong or badly layered.

  • some similar-looking CSS appears in simpleFindWidget.css and in findWidget.css. A decision should be done if these are two separate widgets or a single widget used in different ways. If these are two different widgets, then the workbench specific widget should move to the /workbench/ folder, together with its CSS and images and all. If you'd want to reuse some code, then that's also fine, but then there should be no exact CSS rules that end up e.g. inlining the same image in multiple places in workbench.main.css. As it appears right now, we are somewhere in the middle.

@alexdima alexdima added the debt Code quality issues label Aug 28, 2017
@alexdima alexdima changed the title Duplication in simpleFindWidget Duplication/Layering in simpleFindWidget Aug 28, 2017
@rebornix rebornix added this to the September 2017 milestone Aug 28, 2017
@alexdima alexdima modified the milestones: September 2017, On Deck Sep 28, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

2 participants