Update: Merged a few more patches for Wikimedia
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/1019359
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/1017276

Made a patch for this bug. Here's the relevant screencast:

In T353437#9694492, @Novem_Linguae wrote:

I see a bunch of bugs in your video. Such as duplicating that {{notability}} tag (should detect that it is there and not place it twice, T41319). Or not putting the existing tags in {{multiple issues}} (T361988). This whole section of the code needs a lot of work.

But those bugs are unrelated to this specific ticket and can be handled in other tickets.

Your changes look good. I gotta run, but I think this patch will be merged soon. Thanks for your work on it.

Thanks for the review , I will look into the associated tickets and figure out a fix for them.
Would also appreciate review on the patch I made for T336604 as well, if time allows.

In T353437#9694492, @Novem_Linguae wrote:

I see a bunch of bugs in your video. Such as duplicating that {{notability}} tag (should detect that it is there and not place it twice, T41319). Or not putting the existing tags in {{multiple issues}} (T361988). This whole section of the code needs a lot of work.

But those bugs are unrelated to this specific ticket and can be handled in other tickets.

Your changes look good. I gotta run, but I think this patch will be merged soon. Thanks for your work on it.

Thanks for the review , I will look into the associated tickets and figure out a fix for them.

(Update) Modified the patch for multiple tagging as well and also tweaked the tests. Here's a video:

Should any other changes be made?

In T353437#9694086, @Novem_Linguae wrote:

I'd recommend fixing both in the same patch, if you're willing.

The current patch is breaking a Jest test involving {{Multiple issues}}, so we kind of have to fix it in the same patch anyway.

Although I do think we need to make one change to the failing test too. Should remove the \n before {{Multiple issues.

		return toolbar.submit().then( () => {
			expect( applyTags ).toBeCalledWith( '\n{{Multiple issues|\n{{disputed|date=today}}\n{{linkrot|date=today}}\n}}\nThis is a page.', [ 'disputed', 'linkrot' ] );
		} );

A patch like yours that just changes Wikicode is a great candidate for TDD. You could write a couple extra tests, then just tweak your code until they all pass.

Here's my notes on how to run Jest tests locally: https://en.wikipedia.org/wiki/User:Novem_Linguae/Essays/Docker_tutorial_for_Windows_(WSL)#Jest

Hey! I have made a patch for this issue and it fixes line break for single tags :
.

I have made a fix for this bug. Here is the relevant screencast:

@Yaron_Koren , my apologies I seem to have misunderstood the original bug , should I make a separate task for this new bug and point my patch to it or club the fix for original bug with existing patch? Meanwhile I will work on a fix for this bug.

@Yaron_Koren on inspecting the CSS , element.style adds an extra top of 10px which is dissolved on reloading , so I have made a fix for that which does it immediately however it also subtracts the 10px from the initial comments without resolving.
Before :

After:

@Yaron_Koren there are multiple approaches for this fix , one such approach is using Discussion Tools extension along with Visual Editor and Echo , I feel using visual editor but hiding the toolbar defeats the purpose of the rich-text editor extension but it is needed for Discussion tools and echo extension is already integrated with it via topic subscriptions feature. Whereas on using a 3rd party autocompletion library like https://jqueryui.com/autocomplete/ etc we would need to populate the user data. Which approach would you suggest to be more favourable or would you suggest a custom approach?

@Novem_Linguae I made a patch for fixing this issue , here is the relevant screencast .

Thanks a lot @Jdlrobson for your help , I will definitely keep the inputs in mind for future contributions.

@Jdlrobson I have updated the patch according to the requested changes. Please review.

@Yaron_Koren thanks for the feedback, I have updated my merged patches along with some mock UI in the proposal. I think I can add more information and make it more detailed in the coming days.

@Jdlrobson Thanks that worked , I have made a patch for the fix. Here is the relevant screencast:

From my guess here , isWideScreen() is not updated on browser resize but rather on page reload

From my guess here , isWideScreen() is not updated on browser resize but rather on page reload

@Jdlrobson thanks , that did the job , however I'm not seeing my local changes reflect on the browser for some reason , I followed this command as well

@Novem_Linguae please review the patch. Here is the relevant screenshot

Does something like this look correct to you

@Jdlrobson my Special:MobileOptions on mediawiki docker setup looks like this

@matmarex how can I populate my local database with existing data to test out if my fix is working or not?

@Novem_Linguae can you assist me on how the links are being converted and which section to look out for, as on searching I figured out the parser class is responsible for converting syntax to html if that's right?

@Jatinder190124 you can also review the PRs and see the approved changes by the maintainer that have been made to resolve the issues and open PRs are also a part of contribution but yes I should mention that they are open, I'll make the changes. Thanks.

@Yaron_Koren I think I have figured out an implementation for this issue . I declared an AddTimestamp api which fetches timestamp from wfTimestampnow() and formats it in the same format as AnnotationFormatter.php and displays it with the author name and registered the api in extension.json . I have attached a screencast of the feature , should I make a draft PR as of now as more changes to AnnotationFormatter are yet to be done, also this is for makeComment.js , I believe similar changes have to be made for addReply.js as well.

@Yaron_Koren , @Bawolff please have a look at my GSOC proposal and feedback on it will be really appreciated https://phabricator.wikimedia.org/T359889. Thanks!

@Soda , I already have mediawiki setup and will proceed with the PageTriage Extension installation . I have previously contributed to WikiEduDashboard and InlineComments Extension under Wikimedia, that being said , will my previous contributions help my proposal being selected?

@Yaron_Koren , on going through the codebase I understood a few things ,

1. In makeComment.js the below code snippet gets the username and appends it to the textDiv when a comment is added.

https://github.com/WikiEducationFoundation/WikiEduDashboard/pull/5661
https://github.com/WikiEducationFoundation/WikiEduDashboard/pull/5601
https://github.com/WikiEducationFoundation/WikiEduDashboard/pull/5687
https://github.com/WikiEducationFoundation/WikiEduDashboard/pull/5581 [Draft]
These are some of the PR's that I have made for WikiEduDashboard .

@Yaron_Koren , will my previous contributions to other projects like WikiEduDashboard under Wikimedia help my application in being accepted for this project?

@Yaron_Koren and @Bawolff I went through the extension on my local and currently an option to edit and delete comments is not available (a similar example , say google docs has those features) , we can go around that by resolving the comments but introducing these features will make it more user friendly . What are your opinions on it?

Thanks a lot for your help @Yaron_Koren . I previously made some contributions to WikiEduDashboard and found this project listed for GSOC quite interesting. Should I try my hands on some of the bigger tasks like T355943 to get a better understanding of the codebase or would you suggest some other tasks for me to take up?

In T356436#9599720, @Bawolff wrote:

In T356436#9599232, @Rockingpenny4 wrote:

@Yaron_Koren I configured the InlineComments extension on my local and went through the documentation and found out that it uses ContentHandler , however I was not able to figure out how to navigate to it on my localhost. Is there any reference for that I can go through?

Are you able to navigate to MediaWiki (If not, the answer depends on how you setup mediawiki, see https://www.mediawiki.org/wiki/How_to_become_a_MediaWiki_hacker for more info)? You would just go to the main page of your wiki once set up. At the start there are no comments so you have to add one by highlighting text. You can only do this if you logged into an account, so make sure you did that.

@Yaron_Koren I configured the InlineComments extension on my local and went through the documentation and found out that it uses ContentHandler , however I was not able to figure out how to navigate to it on my localhost. Is there any reference for that I can go through?

@Yaron_Koren I resolved the errors and now the composer test is not showing any linting errors, I have updated the patch as well.

I went through the pipeline failure and there were some linting errors and I tried to fix it using

phpcbf --standard=PSR12  extensions/AdminLinks/includes/ALRow.php

I have updated the changes for it.

Any of 'data-rowName' , 'data-row-name' would be clear and simple enough I think.

I thought name was meant to be this->name of the row, so should I leave it as it is or add name? And I will fix the tabs and spaces.

In T327263#9596129, @Yaron_Koren wrote:

@Rockingpenny4 - great, this looks like a reasonable approach to use.

Two things:

• Please use tabs instead of spaces.
• Using simply the row name as the class name is not a good solution, I don't think - there's a high chance that a row name like "main" will conflict with some other class called "main". I think there are only two real approaches: having it as a class name with a prefix like "admin-links-row-", or having it as a custom attribute like "data-row-name". They both have their advantages; I'm not sure which is the better solution. What do you think?

@Yaron_Koren I have made the required changes and tested them locally and updated on the patch.

@Yaron_Koren
I will definitely use tabs and make the indentation changes , both approaches will work but I would prefer custom attributes as it would make it easier for future developers to maybe add more information to rows and also convenient for designers.

Apologies for the inconvenience , I'm still figuring out gerrit , it looks like I made the changes but didn't click publish, here are the new changes : https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdminLinks/+/1007973/3 .

@Yaron_Koren https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AdminLinks/+/1007973

I changed the approach a bit as in the earlier approach, rawElement self opened and closed the p tag and another p tag was manually appended resulting in two p tags. In the new approach all the content is collected first inside the loop and passed as an argument. This ensures that the <p> tag is properly closed with all the content inside it.

I have attached screenshots above when spaces are included, it replaces spaces with underscore in class names and the website and the source HTML renders fine, should I attach a screenshot of that as well?

Okay then, are there any more changes to be made? If not, I will look into the InlineComments codebase and resolve some of the bigger issues.

@Yaron_Koren I tested row names with spaces and special characters and it breaks with apostrophes so either we can specify to replace apostrophes with something else in classnames or we can use htmlspecialchars to escape them .

@Yaron_Koren I have updated the original patch and while testing it locally, I created an instance of a row with name containing spaces but I wasn't able to explicitly access the class attribute's value .

@Yaron_Koren I have updated the original patch and while testing it locally, I created an instance of a row with name containing spaces but I wasn't able to explicitly access the class attribute's value .

Great, should I go ahead and push the changes?