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

Modify drag'n fill command behaviour to fix audreyt/ethercalc#769. #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

herve0742
Copy link

Hello,
With this pull request I address two points :

Drag'n fill

I think I fix this issue : audreyt/ethercalc#769 by modifying the fillright and filldown command string to embed the increment amount.
My tests consists of running an ethercalc instance and using the fill command in all directions with one, two or more cells first selected.
It seems to me that this is good.

Gulp

I also update gulp package from 3 to 4, to use with newer nodejs.
So I had to adapt gulp.js.
This don't resolve the large warning flow nor the error as on master branch.

@@ -25090,6 +25064,8 @@ SocialCalc.SpreadsheetControlExecuteCommand = function(obj, combostr, sstr) {
var spreadsheet = SocialCalc.GetSpreadsheetControlObject();
var eobj = spreadsheet.editor;

console.log("SCEC : combostr : " + combostr + "\n\tsstr : " + sstr); // DEBUG STUFF
Copy link
Owner

Choose a reason for hiding this comment

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

These probably should be removed

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I agree. How should I do this ? Push a new commit on top of this one or amend this one and force push or something else ?

@marcelklehr
Copy link
Owner

I can't test this atm, so defer to @eddyparkinson's judgement :)

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.

2 participants