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.

Sign up for GitHub

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

Jump to bottom

Open dashboard with code-view shown #478

Merged
merged 1 commit into from Jan 25, 2019
Merged

Open dashboard with code-view shown #478

merged 1 commit into from Jan 25, 2019

Conversation

apoorva17
Copy link
Contributor

Fixes #464

Code view is hidden upon initial loading of report.

Let's open the code view for the author at the top of the ramp chart
upon loading to provide a more representative view of the report,
especially for a user seeing a RepoSense report for the first time.

@damithc
Copy link
Collaborator

damithc commented Jan 14, 2019

Just a thought: to reduce loading time, we can load a dummy page for the code view at first rather than load the code of a specific person.

@eugenepeh
Copy link
Member

Just a suggestion,
wouldn't it be more preferable if its bind to the url instead?
Reason: i can resume to the last author that i'm reviewing.

@damithc
Copy link
Collaborator

damithc commented Jan 15, 2019

wouldn't it be more preferable if its bind to the url instead?
Reason: i can resume to the last author that i'm reviewing.

Yes, that is a good idea. Capturing code view configuration into the URL would be a separate feature though. The first step is to open the code view by default when launching the report for the first time, but show a dummy code view page to reduce loading time. The dummy page can have some instructions on how to view the code of a specific person.

@apoorva17
Copy link
Contributor Author

Just a thought: to reduce loading time, we can load a dummy page for the code view at first rather than load the code of a specific person.

I am currently using information of the 1st author of the 1st repo which is already loaded at the time of the creation of the report, so I am not sure how much it would affect the loading time to use a dummy page instead.

@damithc
Copy link
Collaborator

damithc commented Jan 15, 2019

I am currently using information of the 1st author of the 1st repo which is already loaded at the time of the creation of the report, so I am not sure how much it would affect the loading time to use a dummy page instead.

Are you sure? In the current version, loading the code view of a person takes an appreciable time even if it is the first person in the list; I'm not sure how the first one can be loaded by default without any additional delay.

@apoorva17 apoorva17 force-pushed the 464-open-dashboard-with-code-view branch from 81979c5 to 3c64eed Compare January 16, 2019 14:18
@apoorva17
Copy link
Contributor Author

apoorva17 commented Jan 16, 2019 •
edited

I've updated the implementation to show a dummy code view page with some instructions on how to view the code of a specific author.

@apoorva17 apoorva17 force-pushed the 464-open-dashboard-with-code-view branch from 3c64eed to e302c16 Compare January 19, 2019 13:38
@apoorva17
Copy link
Contributor Author

@reposense/devs i've updated the branch ready for review

damithc
damithc reviewed Jan 20, 2019
View reviewed changes
frontend/src/index.jade Outdated
@@ -38,6 +38,11 @@ html
v_authorship(
v-bind:key="generateKey(tabInfo.tabAuthorship)",
v-bind:info="tabInfo.tabAuthorship")
#tab-authorship.tab-pane(v-else)
.title
span To get started, click on the  
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change as follows?
To get started ... To view the code attributed to a specific author, click the </> icon next to that author's name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the empty tab shouldn't be id-ed #tab-authorship, maybe call it #tab-empty or something.

cos the idea is that, next time we can add new kinds of side display, say a diff view will have the id #tab-commit

@apoorva17 apoorva17 force-pushed the 464-open-dashboard-with-code-view branch from e302c16 to 02b9c9f Compare January 21, 2019 02:45
@apoorva17
Copy link
Contributor Author

@damithc @ongspxm requested changes have been made

@yamidark yamidark requested a review from ongspxm January 21, 2019 13:12
ongspxm
ongspxm approved these changes Jan 21, 2019
View reviewed changes
Copy link
Contributor

@ongspxm ongspxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good to go

@yamidark yamidark requested a review from damithc January 22, 2019 18:08
@eugenepeh
Copy link
Member

Just came to my mind... now that the code view is opened by default, perhaps we should also inform the user that the code view can be hidden?

@apoorva17
Open dashboard with code-view shown
06f4750 
Code view is hidden upon initial loading of report.

Let's open a dummy code view with instructions on how to view the code
of a specific author, to guide a user seeing a RepoSense report for
the first time.
@apoorva17 apoorva17 force-pushed the 464-open-dashboard-with-code-view branch from 69f879d to 06f4750 Compare January 24, 2019 11:09
@apoorva17
Copy link
Contributor Author

Just came to my mind... now that the code view is opened by default, perhaps we should also inform the user that the code view can be hidden?

Ok, I've added a line to inform the user that the code view can be hidden

@yamidark yamidark merged commit 2ad638a into reposense:master Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ongspxm ongspxm ongspxm approved these changes

@damithc damithc damithc approved these changes

@yamidark yamidark yamidark approved these changes

@eugenepeh eugenepeh eugenepeh approved these changes

Assignees
No one assigned
Labels
s.ToMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@apoorva17 @damithc @eugenepeh @ongspxm @yamidark @yong24s