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

[Diff mode] Allow clicking on variables that are in modified text #2856

Open
caraitto opened this issue May 16, 2024 · 2 comments
Open

[Diff mode] Allow clicking on variables that are in modified text #2856

caraitto opened this issue May 16, 2024 · 2 comments

Comments

@caraitto
Copy link

  1. Modify a spec, build the diff. For instance, click on the "Diff" link in the "Preview | Diff" section of Spec: Fix error and not requested cases around trusted bidding signals WICG/turtledove#1180. (I'm not sure how to do this from the command line, it doesn't seem to be documented -- maybe it's another tool and not Bikeshed that does this, or maybe not? I couldn't figure it out looking at the GitHub build log for that PR.)

  2. Find a yellow section of modified text. You can use the "j" and "k" keys to quickly go to next and previous diffs.

  3. Click on a variable referenced in the yellow text.

EXPECTED: All instances of that variable get highlighted, just like would happen for non-modified text.

RESULT: Nothing happens. However, if that variable is referenced somewhere outside the diff'd region, clicking on that instance will highlight all occurrences, including those in the diff'd region.

@caraitto
Copy link
Author

See #2855 -- this may require changing the HTML diff service (which isn't in Bikeshed) to somehow avoid altering the mouseover effect for the diff elements it modifies. This bug probably can't be fixed via changes to Bikeshed code?

@tabatkins
Copy link
Collaborator

Yup, the diff service is totally unrelated to Bikeshed; it's https://services.w3.org/htmldiff. That service would need to change its diff styles.

Probably it wants to make the background change for ins/del dependent on whether an ancestor has data-var-color attribute, which is what Bikeshed uses to trigger variable highlights.

For example, something like:

var[data-var-color] ins.diff-new {
  background: repeating-linear-gradient(135deg, var(--diff-new-bg) 0 3px, transparent 0 10px);
}

However, the behavior where clicking on a var inside a diff section doesn't work, that's fixable by me. I'm currently checking that the clicked element is the var, but when the diff content is applied, the var contents are wrapped in an ins, so clicking the text actually clicks the ins and fails to trigger the highlighting. That's fixable.

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

No branches or pull requests

2 participants