fixed row and column subtract button values for issue #755 #757

NARUDESIGNS · 2021-11-11T14:47:44Z

Fixed issue with the row and column subtract buttons. These buttons when clicked previously yielded a negative text value in the button. However, with the new fix, the text values shouldn't be any less than 1 row and 1 column as a table generally should have at least 1 row and 1 column.
Please help review and feedback @TildaDares @jywarren
Thank you.

welcome · 2021-11-11T14:47:46Z

Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help.
Please make sure you've bumped up the version number in the repository's package.json file and that you've incorporated your changes in the /dist/ sub-directory.
Dangerbot will test out your code and reply in a bit with some pointers and requests.
Also please refer here for installation help 💿
There may be some errors, but don't worry! We'll work through them with you! 👍🎉😄
It would be great if you can tell us your Twitter handle so we can thank you properly?

gitpod-io · 2021-11-11T14:47:47Z

TildaDares

Hi @NARUDESIGNS, your formatter/linter has added more changes than required. Do you think you could disable your linter and then undo the changes it added? Thank you!

TildaDares · 2021-11-11T15:09:19Z

src/modules/PublicLab.RichTextModule.Table.js

  $(document).on('click', '#decRows', function() {
-    $("#tableRows").text( Number($("#tableRows").text()) - 1 );
+    if (Number($('#tableRows').text()) > 1) {
+      $('#tableRows').text(Number($('#tableRows').text()) - 1);


I think the Number($('#tableRows').text()) value should be assigned to a variable so that there's less repetition. The same thing applies to lines 76-77. Thanks!

Also, you need to run grunt build to compile into /dist/PublicLab.Editor.js

OK thank you. Do I have to push the dist/PublicLab.Editor.js changes as well?
I only pushed the file where I added those if conditions...

Yes you also have to push the changes in the ‘dist’ folder.

NARUDESIGNS · 2021-11-11T16:45:57Z

Hi @NARUDESIGNS, your formatter/linter has added more changes than required. Do you think you could disable your linter and then undo the changes it added? Thank you!

OK I will, thanks for pointing out.

This reverts commit 895574e.

… changes

NARUDESIGNS · 2021-11-12T23:30:22Z

@TildaDares I've reverted the changes, disabled prettier (my formatter), created a variable and made the changes. Please review and feedback.
Thank you so so much.
Happy weekend!

TildaDares · 2021-11-15T07:01:14Z

dist/PublicLab.Editor.js

    $(".wk-commands, .wk-switchboard").addClass("btn-group");
    $(".wk-commands button, .wk-switchboard button").addClass(
-        "btn btn-light"
+        "btn btn-outline-secondary"


Hi @NARUDESIGNS, is this change related to this PR?

Everything else looks great btw. Great job!

@TildaDares this change happens automatically when I work on src/modules/PublicLab.RichTextModule.Table.js and I don't know why. Do I need to take it out manually?

@NARUDESIGNS Undo the changes and then do a push. Let's see if that works.

I should undo the changes just for dist/Publiclab.Editor.js right?

@TildaDares this change happens automatically when I work on src/modules/PublicLab.RichTextModule.Table.js and I don't know why. Do I need to take it out manually?

Also just to update you, this automatic change (addition of "btn btn-outline-secondary" property) in the dist/Publiclab.Editor.js happens in my local env just before I push to remote repo this is why I was asking you if I need to manually remove the change.
@TildaDares

Hi @NARUDESIGNS, I'm not sure why but that change is added every time you run grunt build.

The problem is with this commit b1cb346. grunt build was supposed to be run after adding the change so every time anyone else runs grunt build, it adds that change to the dist folder.

Leave the change in one of your PRs and remove it from the other. All you have to do is undo just that line of change after running grunt build.

Ok, thank you very much. I have removed it after running grunt build for PR #758 while the change remains here on PR #757 just like you advised.
Please review, thank you so much for your time @TildaDares

TildaDares

Great job @NARUDESIGNS. LGTM!

Could you add a screen recording of the change just to make sure everything works as it should? Thank you!

NARUDESIGNS · 2021-11-17T20:54:08Z

Sure, not a problem. I will!
Thank you for your time.
@TildaDares

NARUDESIGNS · 2021-11-19T18:37:53Z

Hi, @TildaDares here's a short screen record to show you how it behaves now.
With the resolution, the number of rows or cols can not be less than 1.
However, I'd like your opinion on whether we need to set max rows and cols as well (which I think we should do).
Thank you.

publiclab_sub-btn.mov

jywarren · 2021-12-07T15:17:10Z

This one is nice and simple, so let's merge it! It's not complex enough to need a test. Thanks!

welcome · 2021-12-07T15:17:21Z

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to PublicLab.org in the next few days. In the meantime, can you tell us your Twitter handle so we can thank you properly?
Do join our weekly check-in to share your this week goal and the awesome work you did 😃.
Reach out to someone else working on theirs on Public Lab's code welcome page. Thanks!
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

Help others take their first step

Now that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌

https://code.publiclab.org

Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕

People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉

Read about how to help support another newcomer here, or find other ways to offer mutual support here.

NARUDESIGNS · 2021-12-07T17:25:38Z

Merged!
Thank you!!!!
@TildaDares @jywarren

fixed row and column subtract buttons

895574e

TildaDares requested changes Nov 11, 2021

View reviewed changes

NARUDESIGNS added 2 commits November 12, 2021 23:19

Revert "fixed row and column subtract buttons"

d3f03fb

This reverts commit 895574e.

fixed rows and cols subtract buttons and reverted formtter (prettier)…

45aeea1

… changes

used variable in decCols to avoid repetition

cb4033c

TildaDares reviewed Nov 15, 2021

View reviewed changes

TildaDares mentioned this pull request Nov 15, 2021

Added style to col subtract button to fix issue #756 #758

Merged

TildaDares approved these changes Nov 17, 2021

View reviewed changes

NARUDESIGNS mentioned this pull request Nov 30, 2021

Column subtract button in table popover has no style #756

Closed

jywarren approved these changes Dec 7, 2021

View reviewed changes

jywarren merged commit ac77f82 into publiclab:main Dec 7, 2021

NARUDESIGNS mentioned this pull request Jan 29, 2022

[Planning] Refine the Public Lab Rich Editor #760

Open

31 tasks

fixed row and column subtract button values for issue #755 #757

fixed row and column subtract button values for issue #755 #757

Uh oh!

Conversation

NARUDESIGNS commented Nov 11, 2021

Uh oh!

welcome bot commented Nov 11, 2021

Uh oh!

gitpod-io bot commented Nov 11, 2021

Uh oh!

TildaDares left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NARUDESIGNS Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NARUDESIGNS commented Nov 11, 2021

Uh oh!

NARUDESIGNS commented Nov 12, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TildaDares left a comment

Choose a reason for hiding this comment

Uh oh!

NARUDESIGNS commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NARUDESIGNS commented Nov 19, 2021

Uh oh!

jywarren commented Dec 7, 2021

Uh oh!

welcome bot commented Dec 7, 2021

Uh oh!

NARUDESIGNS commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NARUDESIGNS Nov 11, 2021 •

edited

Loading

NARUDESIGNS commented Nov 17, 2021 •

edited

Loading