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] Move report generation timestamp #2241

Closed
wants to merge 3 commits into from

Conversation

jedkohjk
Copy link
Contributor

Resolves #1832

Proposed commit message

Shift report generation timestamp up

Removes footer so that there is no inconsistency regarding 
the presence/absence of a footer between panels.

Rephrases message so it does not look like it is part of title.

Other information

Before:

before-top

before-bottom

After:

after-top

after-bottom

@damithc
Copy link
Collaborator

damithc commented Aug 1, 2024

Thanks for the PR @jedkohjk
hmm... I don't think the timestamp/version etc. fits in the top/middle of the report either, as it is just a 'small print' that doesn't deserve space in high-value screen real estate.

@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 1, 2024

Thanks for the review @damithc

Hmm, I agree that the version and user guide would fit a duplicated footer, as suggested in the original issue. But I felt like the timestamp may not feel like it fits on the right panel as the timestamp is for the entire report. I'm not sure if that makes sense.

As of now, I don't really have any other suggestions. Perhaps we could shift the reposense version and user guide back to the footer and duplicate it, and leave the time stamp on top?

Something like this (only for the left panel, the bottom of the right panel should have the reposense link in what I'm proposing, but I made this with microsoft paint):

Screenshot

Without the blue hyperlinks, the timestamp is less attention grabbing.

@ckcherry23
Copy link
Member

CS2103/T students may prefer finding the timestamp more easily on the top/middle as it can help them figure out why certain commits have not been updated yet.

However, a timestamp at the top may not be ideal for our other use cases, such as creating a project code portfolio.

@jedkohjk jedkohjk changed the title Move report generation timestamp [#1832] Move report generation timestamp Aug 3, 2024
@jedkohjk
Copy link
Contributor Author

jedkohjk commented Aug 3, 2024

Hi, I made a separate pull request, #2243 as another alternative. There, I left the timestamp as a footer on the left panel, and duplicated only the reposense links.

@jedkohjk
Copy link
Contributor Author

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

@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
3 participants