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

Improves "save as Excel" functionality #2266

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

ckaczor
Copy link
Contributor

@ckaczor ckaczor commented Oct 10, 2023

  • Adds option to bold the header row
  • Adds option to freeze the header row
  • Adds option to enable auto filtering on the header row
  • Adds option to automatically size columns based on content

- Adds option to bold the header row
- Adds option to freeze the header row
- Adds option to enable auto filtering on the header row
- Adds option to automatically size columns based on content
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

As part of updating the dependencies in Packages.props we require that any PRs opened also verify that
they've done the following checks.

Please respond to this comment verifying that you've done the appropriate validation (or explain why it's not necessary) before merging in the PR

  • Built and tested the change locally to validate that the update doesn't cause any regressions and fixes the issues intended
  • Tested changes on all major platforms
    • Windows
    • Linux
    • Mac
  • Check the source repo for any open issues with the release being updated to (if available)

@Charles-Gagnon
Copy link
Contributor

@kisantia Something for you to look at for consideration.

@kisantia
Copy link
Contributor

@cheenamalhotra can you please take a look and consider this?

@ckaczor can you please add screenshots of how this looks and how the options will be exposed in ADS?

@ckaczor
Copy link
Contributor Author

ckaczor commented Oct 11, 2023

The corresponding ADS code changes are here: https://github.com/ckaczor/azuredatastudio/tree/save-as-excel

This is how the options look in the UI:

ADS Options

I had wanted to have the settings layout and behavior more dependent on the "include headers" checkbox but I couldn't find any way to do that with the current ADS code.

@ckaczor
Copy link
Contributor Author

ckaczor commented Oct 11, 2023

As part of updating the dependencies in Packages.props we require that any PRs opened also verify that they've done the following checks.

Please respond to this comment verifying that you've done the appropriate validation (or explain why it's not necessary) before merging in the PR

  • Built and tested the change locally to validate that the update doesn't cause any regressions and fixes the issues intended

  • Tested changes on all major platforms

    • Windows
    • Linux
    • Mac
  • Check the source repo for any open issues with the release being updated to (if available)

I don't have a Mac system for testing but I confirmed that SkiaSharp supports Mac systems.

Copy link
Member

@cheenamalhotra cheenamalhotra left a comment

Choose a reason for hiding this comment

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

Hi @ckaczor

Thanks for contributing this work!

Please proceed to open peer PR in ADS repo with the changes in https://github.com/ckaczor/azuredatastudio/tree/save-as-excel branch and address minor feedback. Looking forward to getting the new features out in next ADS release!

cc @erinstellato-ms for visibility.

@cheenamalhotra
Copy link
Member

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

- Fail tests if expected exception did not get thrown
- Add documention of columnCount parameter for ExcelSheet constructor
- Add element comments to new (and existing) WriteEndElement calls
@cheenamalhotra cheenamalhotra merged commit ad0aedb into microsoft:main Oct 11, 2023
6 checks passed
@ckaczor ckaczor deleted the save-as-excel branch October 12, 2023 11:19
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.

4 participants