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

add display attribute to hyperlinks #2791

Merged
merged 1 commit into from
Sep 23, 2022
Merged

Conversation

phaseOne
Copy link
Contributor

@phaseOne phaseOne commented Sep 14, 2022

Issue

This resolves an import issue with Google Sheets where the cell's text content is lost. On import, Sheets converts hyperlinks to a HYPERLINK formula. When the <hyperlink> lacks a display attribute, it drops the text content of the cell and replaces it with the location of the hyperlink.

Example

An XLSX with the following <hyperlink>:

<hyperlink ref="E2" location="'Sheet1'!A3"/>

When imported into Google Sheets, E2 becomes:

=HYPERLINK("#gid=1368866318&range=A3","#gid=1368866318&range=A3")

Change

This change automatically sets the display attribute of a <hyperlink> based on the cell's value.

I don't see much of a reason to expose the display attribute in the API like the Tooltip property, since it "should" always be the cell's value.

I've only made this change to the XLSX format renderer. Contributions for XLSB and other formats, tests, and feedback on my implementation are welcome.

Quirks Observed

  1. When opening an XLSX in Excel generated by this package (before this PR), Excel natively sets the display attribute of all <hyperlink>s on save, even if they previously did not have this attribute.

  2. Excel will render the underlying text content of the cell and ignores the display attribute of the <hyperlink>, even if they differ. Google Sheets clobbers the text content and only uses the display attribute to set the HYPERLINK formula's link label.

automatically sets display based on the cell's value per Excel behavior
resolves an import issue with Google Sheets
@SheetJSDev
Copy link
Contributor

Google Sheets does not currently import XLSB but does import XLS (so we'd need to check that behavior, but that can happen in a different issue/PR)

External links seem to work without the attribute in Google Sheets (you can try the examples on https://docs.sheetjs.com/docs/csf/features/hyperlinks) but we can reproduce the issue with internal links.

Did you test if the patch works with same-sheet links (e.g. ws["C1"].l = { Target: "#E2" };)

@SheetJSDev SheetJSDev merged commit 1ca49a5 into SheetJS:master Sep 23, 2022
@SheetJSDev
Copy link
Contributor

Testing today (2022-09-23):

  • External links to sites work without display
  • mailto links work without display
  • Local links are lost (irrespective of display setting)
  • Internal single-cell / range / name / scoped references are rewritten without display

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.

2 participants