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

TS and JS inconsistencies #120

Open
svipas opened this issue Apr 23, 2019 · 10 comments · May be fixed by #142
Open

TS and JS inconsistencies #120

svipas opened this issue Apr 23, 2019 · 10 comments · May be fixed by #142

Comments

@svipas
Copy link

svipas commented Apr 23, 2019

Interpolated string literals:

It's not working in TS because it doesn't contain source.ts and source.tsx here: https://github.com/arcticicestudio/nord-visual-studio-code/pull/114/files

JSDoc:

TS
ts

JS
js

In my opinion it should be as it's in TS and type annotations like string should be colored.

@svipas svipas changed the title TS interpolated string literals TS and JS inconsistencies Apr 23, 2019
@arcticicestudio
Copy link
Contributor

Thanks for your contribution 👍

This is a known problem and a note has also been added to the backlog to fix the missing syntax scopes.
I'll take this issue as reference instead of converting the note to an issue 😄

@svipas
Copy link
Author

svipas commented Jun 8, 2019

Also since it's related I found another issue with class initialization.

Screen Shot 2019-06-08 at 22 19 22

As you can see in JS it's not colorized, but in TS it is.

@svipas
Copy link
Author

svipas commented Jun 12, 2019

@arcticicestudio I know you're busy and I can help you. If needed I can solve these issues and submit a PR, I can even try to make screenshots before -> after.

@arcticicestudio
Copy link
Contributor

@svipas Unfortunately my free time is indeed very limited and I try to get the transition of all Nord port projects to the new website done when there is some time so I really appreciate every help and contribution even more than usual 😄
Feel free to go ahead and submit a PR, I'll try to find some time on a weekend to review it. Also screenshots are a nice-to-have, but please do not do any extra work to create them, I always create missing screenshots anyway as soon as I write changelogs and prepare the release version deployments.

I guess most existing selectors for JavaScript (source.js) can be extended by adding the source.ts scope, but as far as I've used TypeScript there might also be some new selectors, e.g. to highlight interfaces or the primary or abstract types and any other element that is specific to TypeScript.

For React JSX the selectors should work fine when the .jsx and .tsx scopes are appended for selectors that are relevant and might not be highlighted correctly yet.

@svipas svipas linked a pull request Jun 13, 2019 that will close this issue
@svipas
Copy link
Author

svipas commented Jun 13, 2019

@arcticicestudio Created a PR: #142

@svipas
Copy link
Author

svipas commented Jun 14, 2019

Also, found a bug with meta.brace.round this scope was missing, and if you take a look at colors of () function calls, you will see they are #d8dee9 instead of #eceff4. This is also fixed in the PR mentioned above.

Screen Shot 2019-06-14 at 22 29 41

@svipas
Copy link
Author

svipas commented Jun 15, 2019

Found issue with meta.brace.square too similar as mentioned above and pushed a new commit to a PR. I guess right now everything is OK and there will be no problems :)

@razvanbadileanu
Copy link

razvanbadileanu commented Oct 4, 2022

That's indeed a nice theme and I want to congratulate you for this, but it seems there is a small issue and I'm referring to the same topic. A small one but annoying and it's related again to typescript, in template string expression color representation differs from JS version, this one been more readable.

JS version
JS version

TS version
TS version

@svengreb
Copy link
Member

Thank you for your patience! 🙏🏼
It‘s been a while since I had free time to focus more on Nord, and my open source projects in general, and invest time in this issue due to work-life balance.

I recently published the first “Northern Post — The state and roadmap of Nord“ announcement which includes all details about the plans and future of the Nord project, including the goal of catching up with the backlog. This issue is part of the backlog and therefore I want to triage and process it to get one step closer to a “clean state“. Read the announcement about reaching the “clean“ contribution triage state in Nord‘s discussions for more details about the goal.

Therefore it has been added for triage in the central and single-source-of-truth project board that is also described in more detail in the roadmap announcement.


@svipas Thanks again for your contributions 🚀
The issue has been added for triage and the pull request as well to plan the review for the next iterations!

@svipas
Copy link
Author

svipas commented Jul 17, 2023

@svengreb Thanks for the best theme ever! This PR is pretty old and might require some work to do before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: triage
Development

Successfully merging a pull request may close this issue.

4 participants