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.

Sign up for GitHub

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

Jump to bottom

chore: environment highlight colour schema update #4464

Merged
merged 11 commits into from
Oct 25, 2024
Merged

chore: environment highlight colour schema update #4464

merged 11 commits into from
Oct 25, 2024

Conversation

nivedin
Copy link
Member

@nivedin nivedin commented Oct 22, 2024 •
edited
Loading

Closes HFE-605 #4389

Before

image

After

image

This PR improves the colors on variables for multiple themes and improve contrast on codemirror.

What's changed

The text colour inside the env variable have been set to white and the syntax highlight has been removed from the env variable for commented line.

jamesgeorge007
jamesgeorge007 requested changes Oct 23, 2024
View reviewed changes
Copy link
Member

@jamesgeorge007 jamesgeorge007 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Variables referenced in comments spanning multiple lines are not greyed out.

    image

  • Comments within a string are considered valid, but a variable getting referenced appears greyed out. This is a very niche use case and a relatively lower priority.

    image

We could revisit these in iterations as required based on evaluating the effort.

@nivedin
Copy link
Member Author

nivedin commented Oct 24, 2024 •
edited
Loading

The HoppComments plugin has been remove since the codemirror MatchDecorator cannot match multi line decorator using regex, ref.

A new approach is added where the syntax tree us generated of the view and checks if the tree has a node with BlockComment, if it exists the environment/ predefined highlight and tooltip is disabled.

Both the issues have been fixed, ptal @jamesgeorge007

image

@nivedin nivedin force-pushed the fix/variable-colour-schema-update branch from 60a4337 to 6a61c90 Compare October 24, 2024 15:59
jamesgeorge007
jamesgeorge007 reviewed Oct 24, 2024
View reviewed changes
Copy link
Member

@jamesgeorge007 jamesgeorge007 left a comment

Choose a reason for hiding this comment

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

Are we not highlighting variables referenced in comment blocks?

@jamesgeorge007 jamesgeorge007 force-pushed the fix/variable-colour-schema-update branch 2 times, most recently from 6a61c90 to 42d5a01 Compare October 24, 2024 16:46
@nivedin
Copy link
Member Author

nivedin commented Oct 24, 2024

Are we not highlighting variables referenced in comment blocks?

yup it was removed, I don't think we need to highlight the commented env variable.

@jamesgeorge007 jamesgeorge007 force-pushed the fix/variable-colour-schema-update branch from 93a261a to 817b202 Compare October 25, 2024 07:20
@jamesgeorge007 jamesgeorge007 force-pushed the fix/variable-colour-schema-update branch from 817b202 to f93d046 Compare October 25, 2024 07:26
@jamesgeorge007 jamesgeorge007 merged commit c72ded2 into next Oct 25, 2024
1 check passed
@jamesgeorge007 jamesgeorge007 deleted the fix/variable-colour-schema-update branch October 25, 2024 07:31
@jamesgeorge007 jamesgeorge007 mentioned this pull request Nov 1, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesgeorge007 jamesgeorge007 jamesgeorge007 approved these changes

@liyasthomas liyasthomas Awaiting requested review from liyasthomas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nivedin @jamesgeorge007