ADDS: Feature to decenter text in wysiwyg and markdown mode #521

shreyaa-s-zz · 2020-06-01T15:02:02Z

FIXES: Decentering of a centered text #489
Enables decentering in both modes
Re-aligning center aligned text left aligns it
Selecting center aligned text as a substring and re-aligning the string center aligns the complete string
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt jasmine
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
PR body includes fixes #0000-style reference to original issue #
ask @publiclab/reviewers for help, in a comment below

shreyaa-s-zz · 2020-06-02T06:58:28Z

The local tests are running fine but ci tests are still on hold, any idea what might be wrong? @jywarren @Shulammite-Aso @keshav234156 @NitinBhasneria ?

keshav234156 · 2020-06-02T07:02:14Z

I have made a PR in this regard

shreyaa-s-zz · 2020-06-02T07:06:20Z

#520 ? Let's wait for it to get merged then. Thanks!

shreyaa-s-zz · 2020-06-04T08:35:15Z

@jywarren @emilyashley @VladimirMikulic @keshav234156 @NitinBhasneria @Shulammite-Aso please review.

VladimirMikulic

If you could fix formatting and have a consistent style, that would be great...

Shulammite-Aso · 2020-06-04T12:19:59Z

code looks great @Shreyaa-s do you mind making a GIF??
That would be cool too!

keshav234156 · 2020-06-04T12:33:02Z

@Shreyaa-s Where are the changes made in? in dist/* folder directly?

shreyaa-s-zz · 2020-06-04T12:59:19Z

No changes to the folder. I mentioned the function that I changed.

shreyaa-s-zz · 2020-06-05T12:36:12Z

Some issues related to centering I found while testing #524 #525. Please have a look.

NitinBhasneria

@Shreyaa-s as @keshav234156 said, you have made changes in dist folder this is not a good practice as dist folder will build every time we run grunt due to which the changes will be lost.

VladimirMikulic · 2020-06-06T15:50:40Z

@NitinBhasneria yes, you are right committing dist folder is highly discouraged, but PublicLab folks do it for some reason :(

@Shreyaa-s please run build and commit changes in the dist.

shreyaa-s-zz · 2020-06-06T18:40:46Z

Done. Thank you everyone!

keshav234156 · 2020-06-10T14:13:42Z

Hi @Shreyaa-s, I was reviewing this PR. If we at the start press center button it doesn't do anything. I think it should start writing at the center if no text is selected at the beginning.

cypherean · 2020-06-10T18:28:14Z

Okay @keshav234156 . I'll make another PR for this.

Shulammite-Aso · 2020-06-15T21:25:41Z

This looks great!!🎉

jywarren · 2020-06-17T01:14:18Z

Hi all, great collaboration! Thank you for being so constructive and supportive in your feedback on this PR. You can never use enough emoji 😍 🎉 💯

just noting:

it should start writing at the center if no text is selected at the beginning.

I believe @Shulammite-Aso has suggested this (i may be mixed up) as a kind of state management way of managing formatting for this scenario. Would this be at woofmark or in this repo? Can someone help connect the dots to where that discussion was happening? Just want to be sure we stay coordinated. Thanks!

jywarren

Wow this looks good. @shreyaa-sharmaa what do you think about writing a test to demonstrate this working? In general this is a best practice to secure functionality against future changes and I think at this point we should all start to do this.

It also makes reviewing much easier as we can read the test to confirm what the code is doing!

cypherean · 2020-06-17T12:53:44Z

Hi all, great collaboration! Thank you for being so constructive and supportive in your feedback on this PR. You can never use enough emoji 😍 🎉 💯

just noting:

it should start writing at the center if no text is selected at the beginning.

I believe @Shulammite-Aso has suggested this (i may be mixed up) as a kind of state management way of managing formatting for this scenario. Would this be at woofmark or in this repo? Can someone help connect the dots to where that discussion was happening? Just want to be sure we stay coordinated. Thanks!

I'm not sure if I have come across this discussion before, @Shulammite-Aso would be able to guide us better here. I believe this should be done in this repo. We can add a condition if(!chunks.selection) before this piece of code.

Shulammite-Aso · 2020-06-17T13:53:40Z

@shreyaa-sharmaa I think this is the comment here #490 (comment) maybe some code inside an if(!chunks.selection) can give us similar behavior, I don't know. You should give it a try. I'm going to try digging into this myself too.

Thanks🎉

cypherean · 2020-06-17T14:28:14Z

@jywarren there's an issue about integrating jest-puppeteer in Editor project like it is in image sequencer #503 which aims to create unit tests for the formatting options. Meanwhile, I'll add a test in a ...spec.js file for this.

cypherean · 2020-06-18T13:43:06Z

The test flow I though of was:

Enter ->Test <- in md textarea
select the text b/w -> and <-
click on center align button
check that the resultant text is Test

Is that right? And feasible?

jywarren · 2020-06-23T20:15:24Z

That sequence for tests sounds GREAT. Thank you!!! 🎉

jywarren · 2020-06-30T16:35:15Z

Hi! I think we are ready for this to get a Jest test now! After #541 is now complete!

cypherean · 2020-06-30T17:45:50Z

I'm working on that test now.

cypherean · 2020-07-02T12:07:01Z

Added the test here. I believe it is failing for the same reason as that mentioned here #543 (comment). Local results for the tests:
After the first click:

At the end:

The local tests:

cypherean · 2020-07-06T09:19:24Z

Travis should pass after merging of #543

jywarren · 2020-07-07T19:32:12Z

Resyncing! If it doesn't pass, perhaps it may need a rebase. Thanks, and great work!

jywarren · 2020-07-14T19:06:33Z

Hi @shreyaa-sharmaa -- can you try rebasing this and hopefully it'll pass? Thanks! 🎉

gitpod-io · 2020-07-14T19:30:40Z

cypherean · 2020-07-14T20:06:39Z

@jywarren this is ready to be merged.

sagarpreet-chadha

excellent!

Just some changes required, also it is good practice to use const instead of var.
Also test is written beautifully! 🎉

src/modules/PublicLab.RichTextModule.AutoCenter.js

cypherean · 2020-07-15T20:18:24Z

Made the required changes. Please have a look @sagarpreet-chadha. Thanks! 😄

cypherean · 2020-07-15T20:21:21Z

excellent!

Just some changes required, also it is good practice to use const instead of var.
Also test is written beautifully!

I used var to maintain consistency in the code since var is used in almost every other module. I can change it to const if that's better?

sagarpreet-chadha

Awesome, looks good 👍

Yes we can use const going forward

jywarren · 2020-07-21T16:52:46Z

Updating branch to then merge! Thanks!

jywarren

Very nice, and love the tests. Thank you!

shreyaa-s-zz mentioned this pull request Jun 3, 2020

Weekly Community Check-In #61 - Bash aliases #523

Closed

VladimirMikulic requested changes Jun 4, 2020

View reviewed changes

NitinBhasneria requested changes Jun 6, 2020

View reviewed changes

NitinBhasneria requested a review from VladimirMikulic June 6, 2020 12:51

shreyaa-s-zz closed this Jun 6, 2020

shreyaa-s-zz force-pushed the decenter_feature branch from 91b66f6 to eefa201 Compare June 6, 2020 14:35

shreyaa-s-zz reopened this Jun 6, 2020

NitinBhasneria approved these changes Jun 6, 2020

View reviewed changes

jywarren changed the base branch from master to main June 16, 2020 22:19

jywarren requested changes Jun 17, 2020

View reviewed changes

cypherean mentioned this pull request Jun 26, 2020

Editor Team - First Release Coordination 🙌🎉 #532

Closed

9 tasks

cypherean force-pushed the decenter_feature branch from 1469be3 to 4be3160 Compare July 2, 2020 11:56

cypherean force-pushed the decenter_feature branch from 33d453f to b487db7 Compare July 14, 2020 19:56

shreyaa-s and others added 4 commits July 15, 2020 01:31

Enables decentering in both modes

b8179ac

changes to the dist folder after build

fe19347

jest test

e10ca45

comments added

664bf5c

cypherean force-pushed the decenter_feature branch from b487db7 to 664bf5c Compare July 14, 2020 20:02

sagarpreet-chadha requested changes Jul 15, 2020

View reviewed changes

required changes and indentation fixes

8c81b51

sagarpreet-chadha approved these changes Jul 17, 2020

View reviewed changes

cypherean and others added 2 commits July 17, 2020 23:21

changes var to const

a8dbbfc

Merge branch 'main' into decenter_feature

166fdef

jywarren approved these changes Jul 21, 2020

View reviewed changes

jywarren merged commit fc4e608 into publiclab:main Jul 21, 2020

jywarren mentioned this pull request Jul 21, 2020

Bump Version from 2.1.2 to 2.1.3, switch to woofmark plots2 branch #557

Merged

keshav234156 mentioned this pull request Sep 1, 2020

GSoC / Outreachy Editor Project Planning Issues and General Disscussion #490

Open

35 tasks

ADDS: Feature to decenter text in wysiwyg and markdown mode #521

ADDS: Feature to decenter text in wysiwyg and markdown mode #521

Conversation

shreyaa-s-zz commented Jun 1, 2020 • edited Loading

shreyaa-s-zz commented Jun 2, 2020 • edited Loading

keshav234156 commented Jun 2, 2020

shreyaa-s-zz commented Jun 2, 2020

shreyaa-s-zz commented Jun 4, 2020

VladimirMikulic left a comment

Choose a reason for hiding this comment

Shulammite-Aso commented Jun 4, 2020

keshav234156 commented Jun 4, 2020

shreyaa-s-zz commented Jun 4, 2020

shreyaa-s-zz commented Jun 5, 2020

NitinBhasneria left a comment • edited Loading

Choose a reason for hiding this comment

VladimirMikulic commented Jun 6, 2020

shreyaa-s-zz commented Jun 6, 2020

keshav234156 commented Jun 10, 2020

cypherean commented Jun 10, 2020

Shulammite-Aso commented Jun 15, 2020

jywarren commented Jun 17, 2020

jywarren left a comment

Choose a reason for hiding this comment

cypherean commented Jun 17, 2020

Shulammite-Aso commented Jun 17, 2020

cypherean commented Jun 17, 2020

cypherean commented Jun 18, 2020

jywarren commented Jun 23, 2020

jywarren commented Jun 30, 2020

cypherean commented Jun 30, 2020

cypherean commented Jul 2, 2020

cypherean commented Jul 6, 2020 • edited Loading

jywarren commented Jul 7, 2020

jywarren commented Jul 14, 2020

gitpod-io bot commented Jul 14, 2020 • edited Loading

cypherean commented Jul 14, 2020

sagarpreet-chadha left a comment

Choose a reason for hiding this comment

cypherean commented Jul 15, 2020

cypherean commented Jul 15, 2020

sagarpreet-chadha left a comment

Choose a reason for hiding this comment

jywarren commented Jul 21, 2020

jywarren left a comment

Choose a reason for hiding this comment

shreyaa-s-zz commented Jun 1, 2020 •

edited

Loading

shreyaa-s-zz commented Jun 2, 2020 •

edited

Loading

NitinBhasneria left a comment •

edited

Loading

cypherean commented Jul 6, 2020 •

edited

Loading

gitpod-io bot commented Jul 14, 2020 •

edited

Loading