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

SetCellHyperlink cannot set display value #790

Closed
jrdallen97 opened this issue Mar 2, 2021 · 2 comments
Closed

SetCellHyperlink cannot set display value #790

jrdallen97 opened this issue Mar 2, 2021 · 2 comments

Comments

@jrdallen97
Copy link
Contributor

Description

The SetCellHyperlink method doesn't allow you to set the display value of the hyperlink. Instead, the library expects us to set the cell value separately to act as the link text. This works fine when using Excel or LibreOffice to view the generated xlsx, but Google Sheets only checks the hyperlink's display value when trying to get the cell text, and falls back to displaying the link location if unset. This means that excelize spreadsheets imported into Google Sheets display wrongly (see below).

GSheet link text not working

For some reason, this only seems to affect location hyperlinks and not external hyperlinks.

Opening the produced file in Excel, saving without making any changes, then importing into Google Sheets causes the link text to work, as Microsoft Excel will set the display value on the hyperlink when saving.


Steps to reproduce the issue:

  1. Create a simple spreadsheet that sets the value of a cell & sets it's hyperlink:
    f := excelize.NewFile()
    f.SetCellStr("Sheet1", "A1", "Location link text")
    f.SetCellHyperLink("Sheet1", "A1", "Sheet1!A5", "Location")
    f.SaveAs("demo.xlsx")
  1. Create a new google sheet, go to File -> Import -> Upload, and upload the produced demo.xlsx.

Describe the results you received:
The cell text is #gid=1908937434&range=A5 and the cell links to cell A5.

Describe the results you expected:
The cell text is Location link text and the cell links to cell A5.


Output of go version:

go version go1.16 darwin/amd64

Excelize version or commit ID:

v2.3.2

Environment details (OS, Microsoft Excel™ version, physical, etc.):
MacOS 11.1 (Big Sur)
Google Sheets
Excel Version: 16.16.27

@jrdallen97
Copy link
Contributor Author

We are willing to create a PR for this, but don't know what the best solution is. We've tested with local version of excelize that adds a displayName argument to SetCellHyperlink which fixes the problem, but would be a breaking change and the spec says that display should be optional. Perhaps the method could accept ...options to optionally accept a display value - this would be backward compatible and would also allow for tooltip to be supported if desired, as it is also marked as optional in the spec?

Diff of the simple change:

diff --git a/cell.go b/cell.go
index 912ee6a..71486e6 100644
--- a/cell.go
+++ b/cell.go
@@ -446,7 +446,7 @@ func (f *File) GetCellHyperLink(sheet, axis string) (bool, string, error) {
 //
 //    err := f.SetCellHyperLink("Sheet1", "A3", "Sheet1!A40", "Location")
 //
-func (f *File) SetCellHyperLink(sheet, axis, link, linkType string) error {
+func (f *File) SetCellHyperLink(sheet, axis, link, linkType, displayName string) error {
        // Check for correct cell name
        if _, _, err := SplitCellName(axis); err != nil {
                return err
@@ -474,7 +474,8 @@ func (f *File) SetCellHyperLink(sheet, axis, link, linkType string) error {
        switch linkType {
        case "External":
                linkData = xlsxHyperlink{
-                       Ref: axis,
+                       Ref:     axis,
+                       Display: displayName,
                }
                sheetPath := f.sheetMap[trimSheetName(sheet)]
                sheetRels := "xl/worksheets/_rels/" + strings.TrimPrefix(sheetPath, "xl/worksheets/") + ".rels"
@@ -485,6 +486,7 @@ func (f *File) SetCellHyperLink(sheet, axis, link, linkType string) error {
                linkData = xlsxHyperlink{
                        Ref:      axis,
                        Location: link,
+                       Display:  displayName,
                }
        default:
                return fmt.Errorf("invalid link type %q", linkType)

@xuri
Copy link
Member

xuri commented Mar 3, 2021

Thanks for your issue. I suggest that to add a ...options to optionally accept the display and tooltip.

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