-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
make "ctrl + c" great again #1379
Conversation
I think its best to try out with the modified example 19. Clicking on a cell, hitting escape and than performing a copy should get you the rendered value of the formatter. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1379 +/- ##
========================================
+ Coverage 99.8% 99.8% +0.1%
========================================
Files 199 199
Lines 21564 21566 +2
Branches 6940 6941 +1
========================================
+ Hits 21501 21503 +2
Misses 63 63 ☔ View full report in Codecov by Sentry. |
gosh you make me think about politics in the south of our border, man I dislike that guy so much 😝 I'll have to give a try later after work, but since you're in this subject, it might of interest to review this SlickGrid issue with Undo. I was waiting for a contribution but the guy never replied back and I never investigated (apart from trying and confirming that it's a bug) |
I can look into the other issue after this PR. I have no idea what that actually is about but we'll find it out ;) with regards to the title, I'll add a sarcasm note next time :) I really didnt find any suitable one since its both a fix and feat with this PR so plz feel free to update to your preference |
hahah no no it's all good, I just wrote what came out of my head at the time of reading lol. Not sure if you saw my other, but out of subject, post in the discussion. Knowing that you're a fan of Prettier, have you read about Biome? I'm not intending to use it in here but I will for sure use it on new project and dump Prettier (in fact I think I will revisit my other projects using Prettier and migrate them to Biome). Only install Biome and get rid of all Prettier & ESLint deps with just 1 Biome dep, config is easy and it's damn fast
|
ups yeah missed the other one. tbh I wasnt quite a fan of the original tooling Rome as it felt just the wrong way to stuff everything in one tool as oposed to the Unix mentality of a single tool for a single purpose. but ultimately I'm not married to either tool. as long as I can press my autofix shortcut and dont have to manually format I'm all for it, whatever its called 😅 |
I also was not sold at all on the Rome idea and never tried it, but I sometime go on the BunJS GitHub for fun just to see what PRs comes in and saw they had a PR last week migrating from Prettier to Biome and decided to give it a try myself. I like the speed but also the setup of installing just 1 dependency (Biome) and not a ton of ESLint/TypeScript plugins to just do 1 thing at the end of the day style+lint, Biome offers that and I like it so far :) Going back in time.. I wonder why they ever deprecated TSLint and decided that following ESLint + a ton of TypeScript deps and config was the best approach... really!?! lol |
@zewa666 I worked late on that project last night so I didn't have a chance to try your PR just yet but I'm just about to test it now |
eslint already had feature parity at that time with tslint, plus it had a way better plugin infra. last but not least new dev slowed down for tslint due to lack of community. I think the unification was actually good. plus you typically only go once through your setup and leave it at that afterwards, so its a pretty small price for the flexibility and autofix support. I'm a big fan of being able to tweak even smaller details like separate library imports from local file imports for readability and eslint made lots of these possible |
yeah the Excel export shouldn't matter but I'm playing it safe... As for the ESLint subject, yes I know all the reasoning behind the move and all the history and it made some sense to merge the 2, however some tools evolve in a way that requires more setup than before (TSLint was really easy to setup). We can also say the same thing for the JS World evolvement, back in the day it was super easy to work with JS, just add a <script> ref and you're good to go... but nowadays you have install WebPack/Vite/Parcel, import the JS lib and finally start coding, I know the "why" I have to do this but still it was easier back in the day to do quick POC. Anyway, I think you get my point, just thinking out aloud 😝 EDIT From your print screen, I guess LibreOffice doesn't support gradient background (it might be an MS Excel only feat), no biggy... Just so you know, I tried to make the UI look like the final export. My main concern was really migrating from JSZip to |
Back to the PR... I'm not sure if it's related or not to the previous SlickGrid issue but the Undo (Ctrl+Z) doesn't seem to work at all, I'm pretty sure it was working at some point but I just tried it in Angular-Slickgrid Example 4 and I don't see it working there either. Is it possible that some of our latest changes broke the Undo? |
Never saw the undo feature so far on this example. Are you sure it was there? The current master definitely doesn't have it. The cell selection not working when an Editor is opened was another bug I've noticed before as well which I'll tackle later down the road. Not introduced by any of these changes. Yeah JS used to be way easier but also way crazier back than. Remember when we had to deal with constructor functions and the revealing module pattern due to lack of visibility modifiers? :) yup LibreOffice doesn't support the full range of features as does Excel |
|
with regards to undo, I remember we talked about a generic undo/redo plugin which users can easily turn on/off. perhaps thats better handled with that instead of recreating logic everywhere. on top of that I agree that example19 starts to become a bit overloaded. |
thats very interesting ... the slickCore.ts issue on line 450 isn't picked up locally but throws by the CI ... |
ah yeah I tweaked the types slightly whenever I see some while reading the code. Does Husky just run when you create the PR? Do we need to add anything to the CI for that? And is your PR ready for final review before merge? I'm making good progress with Excel-Builder-Vanilla, I guess I'll be done by the weekend and give it a try in Slickgrid-Universal afterward... can't wait to see the drop in build size 🍰 |
Husky runs on every git push command. no relation to PRs. my PR is rdy for the final review. |
Just reviewing all comments as well, so I assume this will be addressed later The rest of the PR looks good, I'll go ahead and merge |
@zewa666 I just realized that the title of this PR doesn't have |
yup, conventional commiit messages can be enforced with it. I'll set that up |
as discussed in #1374, here's my approach to both fix the original issue with the missing row/cell ref as well as the new feature to use the formatter even if an editor is defined or grid is editable but the checked cell doesn't currently contain the active editor