-
Notifications
You must be signed in to change notification settings - Fork 157
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
2100 widget info #2238
2100 widget info #2238
Conversation
How should I go about writing test cases for the widget? Edit 1: I have noted that the test cases in optimiseTimeLine only pass if the date indicators are not rendered when the trim timeline is unchecked, should I rename the file and add test cases in the same file too? Edit 2: Should the date indicators still be displayed if there are no commits? Currently, trim timeline does not show the date indicators if there are no commits found. Edit 3: Some frontend test cases that pass on CI are failing locally - how do I fix this? Furthermore, it takes about 40 minutes to finish running the test cases. One example is:
|
Hi @ckcherry23, does this new UI look good to you? Is there any other useful information that could be added to the widget? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @logical-1985516, thank you for looking into this! Not sure if you were able to explore the difference between the main report view and the widget mode, but you can read more about the widget mode here [Widget view, Embeddable widgets].
I don't think we should show the date indicators and "Generated by RepoSense" footers for all ramp charts as seen below in the Report view. This is because the since/until dates would be the same for all, and these dates are mentioned at the top of the report, so it is not that meaningful.
Hence, the date range must be visible conditionally, only when widget mode is true
. Another option for this is to trim/optimise timelines by default for widgets so that the date range is shown. This would also make the widget ramps easier to differentiate/interpret, so I would prefer this.
For now, I think we can remove the "Generated by RepoSense" attribution footer from the centre of the widget as it seems to be too distracting. We can explore adding a small RepoSense logo at the top/bottom right with a link to the RepoSense website.
They don't need to be displayed.
The test case you mentioned is a flaky test known to have issues locally on some devices. Might be issues related to author vs commit date. This has always passed on CI, so we can ignore it. Do let us know if there are any other test cases that are failing. |
Hi, |
This PR was closed because it has been marked as stale for 7 days with no activity. |
Fixes #2100
Proposed commit message
Other information
This is how the new UI looks: