search

wikimedia / WikiContrib

A tool for Wikimedia community members to visualize their technical contributions
https://wikicontrib.toolforge.org
MIT License
23 stars 32 forks source link

title and url added in activity.js #184 #188

Closed shivanigangadharan closed 4 years ago

shivanigangadharan commented 4 years ago

@srish As you mentioned, I tried using only the history.push() function and realised there isn't a need for the onpopstate event. So I have made the change in line 11 of the code. Please ignore the rest of the changes that are shown below, it's all just formatting edits that were automatically done by Prettify installed in my system.

rammanoj commented 4 years ago

Apart of the code segment you added, there is a lot for formatting done, can you please remove it @shivanigangadharan ? (may be add a commit to do so).

shivanigangadharan commented 4 years ago

Apart of the code segment you added, there is a lot for formatting done, can you please remove it @shivanigangadharan ? (may be add a commit to do so).

Okay sure I'll do that and add a commit for the same. Thank you :)

rammanoj commented 4 years ago

One more comment. This is a cool feature to add. But I don't think the code you wrote can solve the complete problem. Your code can solve it partially. Eg:

  1. Create a query with 2 users.
  2. Now you will be displayed with details of first user.
  3. Navigate to second user, and click back button.
  4. You will get back to home page again instead of first user.

It would be better if you can navigate to previous user on clicking back. This can be a bit difficult because all the users have the common URL. The page is getting updated through AJAX requests. Can you index users as 1, 2 .. so on if possible ?

If you want to do the specified change, please don't add it in this PR. As specified in above comment. Remove the formatting. I will merge this. You can create another PR for feature I specifed here.

shivanigangadharan commented 4 years ago

@rammanoj You're right, now I understand what will happen when we create the query with two users, it will keep directing to the home page only. I have removed the formatting changes here, so you may merge this PR. I'll also create a new PR after working on the feature by indexing users as you mentioned. Thank you very much for your guidance.

srish commented 4 years ago

Thanks @shivanigangadharan for submitting this PR! I'm going to merge this now. If you would like to work on @rammanoj's proposed changes, you may submit a new PR :)