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

Worksheet helpers and name to id translation #32

Closed
wojciechczerniak opened this issue Nov 18, 2019 · 9 comments
Closed

Worksheet helpers and name to id translation #32

wojciechczerniak opened this issue Nov 18, 2019 · 9 comments
Labels
API Public methods and properties
Milestone

Comments

@wojciechczerniak
Copy link
Contributor

Description

Mainly our API operates on worksheet numerical ID. Which is OK, but as a developer in Handsontable we have reference to the sheet kept and used mostly as a string name. To integrate those two worlds we will need to read Hyperformula name to id mapping.

The best option here is to add name translation methods:

API proposal

public getSheetID(sheetName: string): number;
public getSheetName(sheetID: number): string;
public updateSheet(sheet: number, { name: string, language: string });
@wojciechczerniak wojciechczerniak mentioned this issue Nov 18, 2019
21 tasks
@wojciechczerniak
Copy link
Contributor Author

We should go further with this API. If we're going to implement full spreadsheet based on HyperFormula then we will need access to more for the UI:

public getSheetCount(): number; // returns the count of worksheets in a workbook.
public doesSheetExists(sheet: number): boolean; // tests whether a worksheet exists.
public getSheetIndex(sheet: string): number; // returns the Index number (position) of a worksheet.
public getSheetName(sheet: number): string; // returns the name of a worksheet.
public getSheetNames(): string[]; // returns a list of all worksheets in a workbook.

@swistak35
Copy link
Contributor

I know that you don't have full sheets support yet, but only a spike, but maybe can you elaborate in which cases do you need to know the sheet name, except of displaying it to the user in the "tabs" below the spreadsheet? Just to have better perspective, I pretty much agree that most of these functions sound useful.

@swistak35 swistak35 self-assigned this Nov 22, 2019
@wojciechczerniak
Copy link
Contributor Author

Sure, no problem.

Any reference to sheets will by name: string. Getting the index was necessary for some APIs which is mostly sorted by #28. But still, it may be helpful to add those helpers. For example: to create simpleCellAddress:

onModifyData(row, col, newValue) {
     return engine.getCellValue({ row, col, sheet: engine.getSheetID(this.sheetName) }
}

(simplified example from handsontable/handsontable#6466)


We don't have the support multiple worksheets fully right now. With the helpers, it will be easly doable by the end-developer. From the end-user or developer perspective we're going to reference sheets mostly by name. Multiple worksheets will be supported by adding an id attribute to Handsontable container. Or by some setting in the settings object. Let's say this is an attribute:

<div class="handsontable" id="sheet1"></div>

And then somewhere lower another one:

<div class="handsontable" id="sheet2"></div>

Inside the plugin, we will take the id and create the engine instance or add a sheet to the existing one:

private initializeThePlugin() {
  if (!privatePool.hyperformula) {
    privatePool.hyperformula = new HyperFormula(this.config);
  }

  const sheetName = this.instance.getSettings().id;
  if (privatePool.hyperformula.isItPossibleToAddSheet(sheetName)) {
    privatePool.hyperformula.addSheet(sheetName);
  }
}

Other methods we will expose to the end developer in our public API. It's up to her to implement the tabs in UI. We don't have to have this functionality in Handsontable. Our clients will appreciate that they have everything that is needed to do it on their own. And IMO this is the preferred approach (at this moment) since everyone has different needs when it comes to the UI.

With getSheetsNames() it's easy to put Handsontable instances in Bootstrap tabs and implement tabbed spreadsheet interface.

@wojciechczerniak wojciechczerniak added this to the December 2019 milestone Nov 28, 2019
@wojciechczerniak
Copy link
Contributor Author

wojciechczerniak commented Nov 28, 2019

Focus is on name translation:

public getSheetID(sheetName: string): number;
public getSheetName(sheetID: number): string;

We can split the rest into a separate task if they are more time-consuming.


Looks like its already is #12

@swistak35
Copy link
Contributor

Most of these are done:

14a2df9

c42c54e

6e59469

2ec72e0

5e10b93

getSheetNames will come. updateSheet is in the form on renameSheet method above. Could you elaborate on what you've meant by language argument? Also, I think we don't need getSheetIndex, as this is purely presentational issue.

@wojciechczerniak
Copy link
Contributor Author

getSheetNames will come. updateSheet is in the form on renameSheet method above.

👍

Could you elaborate on what you've meant by language argument?

Setting the language for the worksheet. But I see now that the language is set once at the workbook level and inherited by worksheets.

Also, I think we don't need getSheetIndex, as this is purely presentational issue.

👍

@wojciechczerniak
Copy link
Contributor Author

We have noticed that sheetId and sheetName might be inconsistent in naming with all other methods. Almost each starts with a verb remove*, add*, set*, get*. Why those two are different?

@wojciechczerniak
Copy link
Contributor Author

wojciechczerniak commented Dec 5, 2019

And an additional question, is the sheetId(name: string): number method case sensitive? YES

After #12 I'm thinking that it should be case insensitive. There shouldn't be two sheets with the same name but different case format. And it would be helpful to find the correct sheetId regardless of a typo in the sheet name.

Extracted to #66

@wojciechczerniak wojciechczerniak added Integration API Public methods and properties labels Dec 5, 2019
@wojciechczerniak wojciechczerniak mentioned this issue Dec 17, 2019
6 tasks
@swistak35
Copy link
Contributor

swistak35 commented Dec 20, 2019

Done in issue #78 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Public methods and properties
Projects
None yet
Development

No branches or pull requests

2 participants