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

feat: datasetIdPropertyName respected in newRowCreator #1279

Merged
merged 2 commits into from Dec 21, 2023
Merged

feat: datasetIdPropertyName respected in newRowCreator #1279

merged 2 commits into from Dec 21, 2023

Conversation

zewa666
Copy link
Contributor

@zewa666 zewa666 commented Dec 21, 2023

Hey ho, when playing around with the Excel Copy Buffer I noticed that in my sample when copying and pasting multiple rows which overflow, new row creation failed with

[SlickGrid DataView] Each data element must implement a unique 'id' property

My example was using the OData Backend hence with a custom id column, set via datasetIdPropertyName.

It took me quite a while to figure out that this can be solved by overriding the following property in the options:

excelCopyBufferOptions: {
     newRowCreator: (count) => { ... }
}

so I thought adding this feature to respect the datasetIdPropertyName should make it work out of the box.

See the added unit test for better clarification :)

@zewa666 zewa666 changed the title Feat/dataset id property name new row creator feat: dataset id property name new row creator Dec 21, 2023
@zewa666 zewa666 changed the title feat: dataset id property name new row creator feat: datasetIdPropertyName respected in newRowCreator Dec 21, 2023
@zewa666
Copy link
Contributor Author

zewa666 commented Dec 21, 2023

I'll be on the lookout for additional issues/convenience features that might arise, so from my perspective dont bother with releasing this asap

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8240c8c) 88.0% compared to head (5fea041) 88.0%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1279   +/-   ##
======================================
  Coverage    88.0%   88.0%           
======================================
  Files         198     198           
  Lines       21361   21361           
  Branches     7102    7103    +1     
======================================
  Hits        18797   18797           
  Misses       2564    2564           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding ghiscoding merged commit 9d60a9d into ghiscoding:master Dec 21, 2023
5 checks passed
@ghiscoding
Copy link
Owner

ah good catch, I almost never use this feature because it conflicts with the Checkbox Row Selection since we cannot have 2 selections model loaded at the same time (excel copy requires cell selection model), however some users found ways to switch between models when mouse clicking and releasing.

I agree that I'll wait a bit more before releasing anything, I also just got Aurelia-Slickgrid working Aurelia 2 as well officially yesterday, so that's a nice Xmas gift in advance 🎁 I also had trouble finding the Angular-Slickgrid issue reported by some users after publishing to AWS and others, anyway it looks to be all fine now, so I think we're good for some time :)

@zewa666
Copy link
Contributor Author

zewa666 commented Dec 21, 2023

the excel like pasting is quite useful for those spreadsheety applications. often customers literally just ask for that in order to move from their precious excel spreadsheets 😅

@ghiscoding
Copy link
Owner

ghiscoding commented Dec 21, 2023

I know the feeling... most users are just expecting a datagrid to behave the same as Excel or if it doesn't then they tag it as being a bad or cheap project lol

Anyway, I'm happy with the journey to modernize the project, I'm pretty much done with what I wanted to achieve and even features that I only dreamed of are now part of the lib (removing jQuery was the biggest of such dream). The only thing that a user recently asked for was Dark Mode, but when I tell him to please contribute, I heard...nothing back as you can expect lol

@zewa666
Copy link
Contributor Author

zewa666 commented Dec 21, 2023

you should be proud of your work if dark-mode is "all thats missing" 😅

to me the biggest selling point, besides the tons of features, is the test coverage. it gives that comfy feeling of knowing things wont fall apart when touching unknown code.

@ghiscoding
Copy link
Owner

ghiscoding commented Dec 21, 2023

indeed the test coverage is really what made it possible to release new major versions with confidence. I know that since a while ago that a lib needs to have proper test coverage to be well recognized. I also made the 6pac/slickgrid more popular since I added few hundreds Cypress tests, there's a lot more users since I dropped jQuery/jQueryUI and converted it to TypeScript, that migration was also the missing piece in order for me to migrate the core files into here and there's a lot more users in that lib now since all of these changes. Some users waited a long time to have full ES6/ESM imports, it's now all possible... woohoo

Happy early Xmas and always enjoyed all contributions from your side, thanks for being a long time user and echoing this to your coworkers, nice to have all these ⭐ too :D

I support Lerna-Lite too if you didn't know, I created it when Lerna wasn't being supported for a while, I use it in Slickgrid-Universal and now in Aurelia-Slickgrid too, it's great for monorepo :)

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.

None yet

2 participants