Skip to content

Prefix sample type menu entry ids with "sampletype-". - #792

Merged
Louis-Ye merged 1 commit into
google:mainfrom
ghemawat:sample
Aug 8, 2023
Merged

Louis-Ye merged 1 commit into
google:mainfrom
ghemawat:sample

Conversation

@ghemawat

@ghemawat ghemawat commented Aug 8, 2023 •

Copy link
Copy Markdown
Contributor

Previously, we would use the sample type name as the HTML id for the corresponding menu entry. However some profiles contain sample type names that conflict with other HTML ids we use (in particular, the sample type name "sample" conflicted with a different id). Remove this name clash by prefixing every menu entry with "sampletype-".

@ghemawat
ghemawat requested a review from Louis-Ye August 8, 2023 20:45
@codecov-commenter

codecov-commenter commented Aug 8, 2023 •

Copy link
Copy Markdown

Codecov Report

Merging #792 (f2511f9) into main (2ba5b33) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #792   +/-   ##
=======================================
  Coverage   66.99%   66.99%           
=======================================
  Files          45       45           
  Lines        9862     9862           
=======================================
  Hits         6607     6607           
  Misses       2795     2795           
  Partials      460      460           

Louis-Ye
Louis-Ye previously approved these changes Aug 8, 2023

@Louis-Ye Louis-Ye left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, with a minor comment.

Comment thread internal/driver/html/common.js Outdated
function setSampleIndexLink(id) {
const elem = document.getElementById(id);
function setSampleIndexLink(si) {
const elem = document.getElementById('sampletype-'+si);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: I think the grammar convention in this file is having space before/after the operator +. Can you change it to that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks.

Previously, we would use the sample type name as the HTML id for the
corresponding menu entry. However some profiles contain sample type
names that conflict with other HTML ids we use (in particular, the
sample type name "sample" conflicted with a different id). Remove this
name clash by prefixing every menu entry with "sampletype-".

some profiles caused the Javascript to break bec

@Louis-Ye Louis-Ye left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Thanks!

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