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

[Playground] Debug links #1179

Merged
merged 22 commits into from
Sep 19, 2024
Merged

[Playground] Debug links #1179

merged 22 commits into from
Sep 19, 2024

Conversation

jamesnw
Copy link
Contributor

@jamesnw jamesnw commented Sep 12, 2024

Description

In the debug console, line numbers are now links to select or position the cursor at the relevant issue. Also, deprecation messages with links are now clickable.

Related Issue(s)

Addresses number 6 in #779.

Steps to test/reproduce

  1. Debug, Warn, and Deprecation links
    2. Error link
  2. Error without a range
  3. Long content, more examples

Links should allow you to open in new tab/window with CMD/Shift/Right click, etc.

Copy link

netlify bot commented Sep 12, 2024

Deploy Preview for sass-lang ready!

Name Link
🔨 Latest commit ee7c1d5
🔍 Latest deploy log https://app.netlify.com/sites/sass-lang/deploys/66eb2cddecea110008579a8b
😎 Deploy Preview https://deploy-preview-1179--sass-lang.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

source/assets/js/playground.ts Outdated Show resolved Hide resolved
source/assets/js/playground.ts Show resolved Hide resolved
source/assets/js/playground.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
}
data.lineNumber = lineNumber;

if (item.type === 'warn' && item.options.deprecationType?.id) {
data.safeLink = `https://sass-lang.com/d/${item.options.deprecationType.id}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the indirection in this safeLink thing a bit confusing, and I worry it'll get worse if we end up wanting to add more HTML to the messages. Consider instead having separate htmlMessage and message variables and injecting htmlMessage ?? encodeHTML(message).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wouldn't work in this situation, unless I misunderstand. I'll explain what I'm doing here a bit more to see if there's a clearer and more future-proof method.

  1. message comes from the Logger, and is inherently unsafe
  2. When we have a deprecationType, we can generate a known safe URL for more info
  3. If that known safe URL exists as a string in message, we want to turn that string into a link.

I realized the link augmentation is a bit confusing to include in the htmlEncode function, so I moved it out in e86f9b9.

@jamesnw jamesnw requested a review from nex3 September 17, 2024 15:06
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
source/assets/js/playground/console-utils.ts Outdated Show resolved Hide resolved
@jamesnw jamesnw requested a review from nex3 September 18, 2024 19:43
@nex3 nex3 merged commit efacf6c into sass:main Sep 19, 2024
9 checks passed
jgerigmeyer added a commit to oddbird/sass-site that referenced this pull request Sep 20, 2024
* main: (23 commits)
  [Playground] Click to Copy (sass#1177)
  Cut a release for a new Dart Sass version
  [Playground] Debug links (sass#1179)
  Add breaking change page for legacy-js-api (sass#1193)
  [Playground] Autocomplete (sass#1166)
  Add a breaking change page for color functions (sass#1192)
  Remove nonexisting `color.saturate()` function (sass#1190)
  Fix unprefixed `color.adjust()` name (sass#1188)
  Sort color functions alphabetically (sass#1187)
  Document color migrator (sass#1186)
  Cut a release for a new Dart Sass version
  Document new color spaces (sass#1055) (sass#1115)
  Cut a release for a new Dart Sass version
  Bump date-fns from 3.6.0 to 4.0.0 (sass#1181)
  Bump rollup from 4.21.2 to 4.21.3 (sass#1182)
  Bump @codemirror/autocomplete from 6.18.0 to 6.18.1 (sass#1183)
  Bump @typescript-eslint/eslint-plugin from 8.5.0 to 8.6.0 (sass#1185)
  Bump markdown-it-anchor from 9.1.0 to 9.2.0 (sass#1184)
  Fix footer not showing the latest versions. (sass#1178)
  [Playground] Code links (sass#1167)
  ...
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.

3 participants