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

Load issues (and PRs) in modals #1293

Closed
favna opened this issue May 2, 2018 · 4 comments
Closed

Load issues (and PRs) in modals #1293

favna opened this issue May 2, 2018 · 4 comments

Comments

@favna
Copy link

favna commented May 2, 2018

It goes without saying that modals ideally take away a lot of loading requirement for the end user and when browsing many issues (i.e. on a repo such as this one) having to load all those individual pages takes either a lot of separate tabs or a lot of wasted time.

That said I want to suggest the feature to instead open issue pages in modals overlaying the scrollable area that GitHub already has for the issues page. An example for this concept can be taken from how the Reddit redesign handles opening posts through modals that are infinitely scrollable and close upon clicking anywhere not inside the modal.

All in all, this will allow for rapid navigation between many issues while still retaining the full view of an issue and its comments.

Taken from /r/git as example image:

reddit_modal

Mockup made by me using this repo's issue tracker at time of writing:

mockup

@notlmn
Copy link
Contributor

notlmn commented May 2, 2018

Modals are only good at showing small amount of information. And showing any content in the modal requires the content to be loaded first, making the user wait all the time the content is being loaded. Modern browsers start displaying content progressively even when they do not have all the resources required to display it.

You can see this effect on GitHub too when loading a page directly or through ajax on a very slow network.

Direct load:

load

Ajax load:

ajax


And to even mimic progressive loading, using iframes would also not be possible as it violates GH's CSP.

@favna
Copy link
Author

favna commented May 2, 2018

Modals are only good at showing small amount of information

That really comes down to opinion IMO. Personally I would disagree with it IF and ONLY IF using modals speeds up the overal experience. That said, I suppose if it's the opinion the contributors to not use modals for a lot of content then that's that, can't wrap around the We're happy to receive suggestions and contributions, but be aware this is a highly opinionated project part I suppose.

Modern browsers start displaying content progressively even when they do not have all the resources required to display it.

I get what you're saying with this, but would that really be different between modals or whole site loads (in a new tab or otherwise)? Regardless whether you open a modal or load the issue in a new tab, it's going to have to load. I have never really delved into the inner workings of browsers these days but the way I would interpret progressively prefetching is that it could use that content for both models and new tags/loading in current tab.

And to even mimic progressive loading, using iframes would also not be possible as it violates GH's CSP.

I never meant for the modals to be iframes, but rather a similar system what Reddit does which would only require JS. Namely, they add a black css background overlay over the page (40-60'ish percent opacity) and add <div>'s for the modal with a higher z-index than the page itself. Taken from Reddit's source code through the web inspector:

image

In the case of GitHub it would require getting the content of the issue that is going to be opened and slamming the most of its body into the overlaying div

@notlmn
Copy link
Contributor

notlmn commented May 2, 2018

Modals are only good at showing small amount of information

That really comes down to opinion IMO

IMO, modals are never good for handling a lot of data. If something is large enough to occupy a lot of browser view, then it should be a page by itself. Comparing this to Tweets, they are mostly read and are only one level deep, but people involve in issues/PRs and discuss a lot, they are one level deep with "Conversation/Commits/Files changed" tabs. And having almost all of DOM nodes in an issue/PR in the listing page is probably a bad idea.


I get what you're saying with this, but would that really be different between modals or whole site loads (in a new tab or otherwise)?

Yes it does, the GIFs are examples for that, ajax loads take lot of time than full page load in terms of First Meaningful Paint (long explainer by @jakearchibald).


I never meant for the modals to be iframes

That was just used as an example of loading content, not as a solution.

@fregante
Copy link
Member

fregante commented May 4, 2018

Why?

Faster opening? No. GitHub is already ajaxed so it takes the same amount of time.

Faster closing? No. Same reason as above.

GitHub is already perfectly-ajaxed so modals don't actually add anything, they just reduce the space available and make things complicated (because suddenly the DOM would have both the issue list and an issue content.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants