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

[#1832] Duplicate RepoSense links #2243

Closed
wants to merge 4 commits into from
Closed

Conversation

jedkohjk
Copy link
Contributor

@jedkohjk jedkohjk commented Aug 3, 2024

Resolves #1832

Proposed commit message

Duplicate RepoSense links on both panels

Separates the report generation time from the links as it is more 
relevant to the left panel.

Added the footer as a component for code reuse.

Changed margins to ensure consistency across both panels.

Other information

Before:

Screenshot

Screenshot

Screenshot)

After:

Screenshot

Screenshot

Screenshot

@damithc
Copy link
Collaborator

damithc commented Aug 4, 2024

Thanks for the PR, @jedkohjk
It is meant to be a footer, to indicate which version was used and what time it was generated. Hence, it contains information not expected to be important to normal users unless someone is specifically interested to know them. In fact, it was added originally to help with debugging, for us to verify that the report is using the correct version of RepoSense and is not showing an outdated version. As a footer, it stays at the bottom edge and inconspicuous, unless the user is specifically looking for those data. Some parts are hyperlinked, but that was for convenience only.
So, splitting it into two and giving each one its own purpose is going beyond what the original footer was intended to do.

As of now, I'm not sure any of the new alternatives proposed in this PR look particularly better than the original. Even duplicating the entire thing on both sides might not look particularly nice but at least it doesn't conflict with the original intention of the footer.

On a related note, footers are normally shown in slightly smaller text than the rest of the page. So, we can consider doing that with out footer as well.

@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 4, 2024

Thanks for the review @damithc
I see. I actually think the link to the reposense webpage and the user guide could be useful to users. Of course, the version linking to a specific commit may come off as a bit odd for users. Maybe making the footer RepoSense (ver.) | User Guide instead of RepoSense | Version | User Guide will make the version less conspicuous to users?

@damithc
Copy link
Collaborator

damithc commented Aug 4, 2024

I actually think the link to the reposense webpage and the user guide could be useful to users.

I agree. For now, we have this:
image

I think there is a better UI design that can fit all these design elements together in a better way. But until we figure what that is, we can keep the current design (with minor tweaks) for the time being.

@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 4, 2024

@damithc Yes. However, the welcome panel disappears upon clicking on a code panel or commits panel. It is not easy to get it back after clicking on these panels. After clicking on one of these panels, the easiest way to access a user guide will be the footer. A user may want to access the user guide after clicking on one of these panels to find out how to interpret the information on the panel.

@damithc
Copy link
Collaborator

damithc commented Aug 4, 2024

Another option is to consider something like the below, at the top of the page.

image

@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 4, 2024

Hmm yes that makes sense. As of now, I'm not sure I know how to implement it though.

@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 11, 2024

Hi, should I close this PR as CP3108A is coming to an end for me?

Also, basued on our discussions, should I mention adding the user guide in the header in issue #1832 for future reference, or create a new issue for it?

@gok99
Copy link
Contributor

gok99 commented Aug 13, 2024

Hi @jedkohjk, yes you may close the PRs if you no longer plan to work on them, and summarize the discussion above (mentioning how it relates to the original issue) in issue #1832, where we can discuss it further.

@jedkohjk
Copy link
Contributor Author

Hi @jedkohjk, yes you may close the PRs if you no longer plan to work on them, and summarize the discussion above (mentioning how it relates to the original issue) in issue #1832, where we can discuss it further.

Alright, will do!

@jedkohjk jedkohjk closed this Aug 13, 2024
Copy link
Contributor

The following links are for previewing this pull request:

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

Successfully merging this pull request may close these issues.

Repeat footer on both panels
4 participants