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

Mark all files as viewed/not-viewed #2444

Closed
umanghome opened this issue Sep 17, 2019 · 32 comments · Fixed by #3007
Closed

Mark all files as viewed/not-viewed #2444

umanghome opened this issue Sep 17, 2019 · 32 comments · Fixed by #3007
Assignees
Labels
enhancement help wanted small Issues that new contributors can pick up

Comments

@umanghome
Copy link

While reviewing PRs with 10+ files, the workflow gets easier by marking files as viewed. However, when I need to re-review the PR, it gets very tedious marking all files as not-viewed before going ahead with the review.

Would be great if we had a button that would mark all files as view or not-viewed.

@fregante

This comment has been minimized.

@Pyton
Copy link

Pyton commented Oct 11, 2019

As temporary solution I wrode JS bookmark:

javascript:boxes=document.getElementsByClassName("js-reviewed-checkbox"),counter=0;for(let e=0;e<boxes.length;e++){const t=boxes[e];"File viewed, click, value:true"==t.dataset.gaClick&&(t.click(),counter++)}alert("Unfolded "+counter);

Just save it as Bookmark and put JS in URL.

@bbugh
Copy link

bbugh commented Oct 22, 2019

I would love this feature. I love the Viewed GitHub feature but the UX is incomplete. (PS thanks for the excellent extension)


If anyone wants a "set all viewed" equivalent of @Pyton's "unview all" snippet:

javascript:boxes=document.getElementsByClassName("js-reviewed-checkbox"),counter=0;for(let e=0;e<boxes.length;e++){const t=boxes[e];"File viewed, click, value:false"==t.dataset.gaClick&&(t.click(),counter++)}alert("Folded "+counter);

@leocaseiro
Copy link
Contributor

I was thinking that a better feature would be on the scroll, as you scroll down the file, it would mark file per file as viewed.

@Pyton
Copy link

Pyton commented Jan 6, 2020

I was thinking that a better feature would be on the scroll, as you scroll down the file, it would mark file per file as viewed.

Nop. Viewed feature (GH) collapse the code so You no longer see the code but only names.

@fregante
Copy link
Member

fregante commented Jan 6, 2020

I was thinking that a better feature would be on the scroll, as you scroll down the file, it would mark file per file as viewed.

Seeing a file is different from being done reviewing it. You might see the whole file in a viewport, but it doesn't mean that you actually read all of it.

The "Viewed" check must be an explicit action

@fregante
Copy link
Member

fregante commented Jan 6, 2020

I'm thinking that we should put a "Mark all as read" button at the end of the page. This makes the most sense to me and also avoids this issue ⬇️, since when you reach the button, everything was loaded:

such feature can only affect files that have already been loaded.

@mfulton26
Copy link

If such a feature can only affect files that have already been loaded then perhaps a "Load All" button/feature should be added first.

If everything isn't loaded the button could be present, otherwise the button isn't shown and other commands are (e.g. "Mark all as read", "Mark all as unread", etc.).

@fregante
Copy link
Member

fregante commented Jan 6, 2020

The solution in my last comment already solves that issue.

@fregante fregante added the small Issues that new contributors can pick up label Jan 6, 2020
@mfulton26
Copy link

Have you ever reviewed a PR that contains 2000+ files? I have. It took for ever just to load all the files and it would have been super nice to have just had a button that would have loaded everything for me instead of me scrolling down over and over again. 😄

@lukasz-tc
Copy link

Have you ever reviewed a PR that contains 2000+ files? I have. It took for ever just to load all the files and it would have been super nice to have just had a button that would have loaded everything for me instead of me scrolling down over and over again. 😄

AFIR when you scroll till loading starts it fetch rest of the code.

@mfulton26
Copy link

mfulton26 commented Jan 6, 2020 via email

@leocaseiro
Copy link
Contributor

leocaseiro commented Jan 6, 2020

In that case, just run the following on your dev tools:

// toggle all to viewed
document.querySelectorAll('.js-reviewed-checkbox').forEach((elem => {
    if (elem.checked) { return }
    var clickEvent = new MouseEvent('click');
    elem.dispatchEvent(clickEvent);
}))

// toggle all to not-viewed
document.querySelectorAll('.js-reviewed-checkbox').forEach((elem => {
    if (!elem.checked) { return }
    var clickEvent = new MouseEvent('click');
    elem.dispatchEvent(clickEvent);
}))

Btw, I shared as a bookmarklet that toggle on/off:
https://codepen.io/leocaseiro/full/bGNaOvM

Drag and drop the link on your bookmarks and click on the bookmarklet on GH PR page.

@fregante
Copy link
Member

fregante commented Jan 7, 2020

Have you ever reviewed a PR that contains 2000+ files?

What's the point of marking all files as viewed if you haven't viewed them?

@mfulton26
Copy link

Have you ever reviewed a PR that contains 2000+ files?

What's the point of marking all files as viewed if you haven't viewed them?

I do review them. I just don't want to click "Viewed" 2,000+ times. 😄

This doesn't happen too often but on some projects it does happen from time to time. I typically like to quickly scan all the files looking for anything suspicious and then just call it good. However, once I review it I'd like to be able to see what changed if someone pushes another commit which only affects a subset of the files, I don't want to have to quickly scan everything again just see what changed.

@fregante
Copy link
Member

The point I'm trying to make is that if you do review them, you also scrolled all the way to the bottom to see the button there.

@Ian1971
Copy link

Ian1971 commented Feb 21, 2020

Just to add a use case. On a large PR we may get all the changes and review them locally in an IDE. In which case mark all as viewed helps a lot.

@fregante
Copy link
Member

Indeed, I didn’t think of checking it via git 👌

@fregante
Copy link
Member

fregante commented Feb 21, 2020

Can you check for an API to mark them as viewed? Marking “2000+ files” means making 2000+ HTTP requests. I think it’s gonna take a lot of time and maybe overwhelm GitHub. You might want to change strategy because GitHub didn’t account for it.

I suggest sending this feature request to GitHub

Maybe, also, avoid such huge PRs. What kind of code are you handing and how can you review that?

@AliSoftware
Copy link

A more interesting feature to me would be to mark all files up to a specific one to be Viewed. Not all files, just all the files from the first one up to one I select.

That way we could have a way of tracking where we were at when we got interrupted while reviewing a super long PR

Perhaps a menu item under the "..." menu of each file like "Mark all files up to and including this one as Reviewed".

To me this is what the "Viewed" feature of GitHub was meant to be: to kind of bookmark where you were at during a review, which means only marking files as Viewed up to a certain one.

@Pyton
Copy link

Pyton commented Feb 24, 2020

@AliSoftware this could be realized by Click select like: click first item, click last item with Shift == Select all between

@pvinis
Copy link

pvinis commented Mar 6, 2020

I'll just add the reason I would like this feature and the reason I made #2865.

Sometimes in big PRs I mark files as viewed when they are finalized for that PR, and leave some unviewed to make sure I clean up before finishing with that PR.

Some other times, while I'm in the middle of working in a big PR, I need to do a "PR Party" and explain the changes to the rest of the team, and to do that it's useful to mask everything unviewed, then with the team go through the files and mark them as viewed as we go. Finally, after that is done, I either uncheck the unfinished views, or uncheck all again and mark the finished files viewed.

@Decat-SimonA
Copy link

Hello, another use-case would be to mark files containing some texts as viewed.
For example, we commit some jOOQ auto-generated code: marking all files containing @Generated as viewed could be useful

@fregante
Copy link
Member

fregante commented Apr 14, 2020

@Decat-SimonA you can use gitattributes to hide/collapse those files like it happens automatically for package-lock.json, for example.

https://github.com/github/linguist#generated-code

"marking all files containing keyword as viewed" is too specific to be part of Refined GitHub.

@mfulton26
Copy link

FYI: I've been using a custom search engine in Google Chrome to mark all files as read/unread, switch to the rich diffs, etc.; this could be modified to use %s in the search URL to take "arguments" to a "Mark all as viewed" function to match file names or contents of files (you'd have to modify the script to detect such, etc.)

Search engine Keyword Query URL
GitHub: Display all rich diffs ghrd javascript: document.querySelectorAll('.js-rendered').forEach(button => button.click());
GitHub: Display all source diffs ghsd javascript: document.querySelectorAll('.js-source').forEach(button => button.click());
GitHub: Mark all as not viewed gh-v javascript: document.querySelectorAll('.js-reviewed-checkbox').forEach(input => !input.checked || input.click());
GitHub: Mark all as viewed gh+v javascript: document.querySelectorAll('.js-reviewed-checkbox').forEach(input => input.checked || input.click());

@fregante fregante self-assigned this Apr 15, 2020
@fregante
Copy link
Member

fregante commented Apr 15, 2020

I'm implementing this via shift-click to mark multiple files at once

@mfulton26
Copy link

I'm implementing this via shift-click to mark multiple files at once

would alt-click not make more sense? e.g. I believe alt-click is already natively supported by GitHub for expanding/collapsing all files

@fregante
Copy link
Member

fregante commented Apr 17, 2020

Alt-click would make sense if it acted on all, but that's not how it works (currently).

Shift-click is the standard for selecting "from X to Y"

I believe alt-click is already natively supported by GitHub for expanding/collapsing all files

That acts when clicking the toggle icon. This feature acts when clicking the "Viewed" checkbox

@refined-github refined-github deleted a comment Apr 22, 2020
@refined-github refined-github deleted a comment Apr 22, 2020
@refined-github refined-github deleted a comment Apr 22, 2020
@refined-github refined-github deleted a comment Apr 22, 2020
@fregante
Copy link
Member

would alt-click not make more sense?

I opened an issue for that: #3043, to extend the feature I implemented.

@Flabbergasted-1

This comment has been minimized.

@lordzsolt
Copy link

I see this PR hasn't moved since it was created 😭

My use case:

  • I review a PR, mark each file individually as "Viewed"
  • PR gets approved and merged
  • After the release, something breaks, so we need to revisit the PR to see what change could've caused it.
  • All files are collapsed, so I individually have to expand all of them....

@fregante
Copy link
Member

@lordzsolt the feature already exists, you just need 2 clicks instead of 1: #3007

@refined-github refined-github locked as resolved and limited conversation to collaborators Feb 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement help wanted small Issues that new contributors can pick up
Development

Successfully merging a pull request may close this issue.