-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
[api-minor] Implement creation/modification date for annotations #10771
Conversation
/botio-linux preview |
/botio test |
Looking briefly at the test results, a couple of things stand out:
|
ce0dac1
to
ba8bc71
Compare
The review comments have been addressed, but I haven't been able to find a way to make localization work in the test driver, which is why we see I don't really see this as a blocker for this PR because we already cover the date parsing in the unit tests. However, a follow-up ticket to find a solution to this seems like a good idea because we never had this use-case before. We also need to fix the CI since after a recent Node.js update the |
775b2f8
to
ba8bc71
Compare
ba8bc71
to
9ed9ba4
Compare
/botio lint |
/botio-linux preview |
/botio test |
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.
This looks generally good; however I do have a couple of comments.
Really sorry about not getting to this PR sooner, since that may have caused you some unnecessary work!
a7eed22
to
940c3ee
Compare
This includes the information in the core and display layers. The date parsing logic from the document properties is rewritten according to the specification and now includes unit tests. Moreover, missing unit tests for the color of a popup annotation have been added. Finally the styling of the popup is changed slightly to make the text a bit smaller (it's currently quite large in comparison to other viewers) and to make the drop shadow a bit more subtle. The former is done to be able to easily include the modification date in the popup similar to how other viewers do this.
940c3ee
to
be1d662
Compare
/botio lint |
From: Bot.io (Linux m4)ReceivedCommand cmd_lint from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bc40373a6129263/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_lint from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/f505a03ac5ca9f9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/bc40373a6129263/output.txt Total script time: 1.01 mins
|
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.
r=me with passing tests; thank you!
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/f505a03ac5ca9f9/output.txt Total script time: 2.84 mins
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/5ddc264023b8a7f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/bdc2565adbd998d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/5ddc264023b8a7f/output.txt Total script time: 17.59 mins
Image differences available at: http://54.67.70.0:8877/5ddc264023b8a7f/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/bdc2565adbd998d/output.txt Total script time: 25.40 mins
Image differences available at: http://54.215.176.217:8877/bdc2565adbd998d/reftest-analyzer.html#web=eq.log |
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/43129aa8d71b6bb/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/43129aa8d71b6bb/output.txt Total script time: 1.89 mins Published |
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/b72fe8f076f1a9e/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/9e1e4a43740a46e/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/b72fe8f076f1a9e/output.txt Total script time: 16.22 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/9e1e4a43740a46e/output.txt Total script time: 23.00 mins
|
This includes the information in the core and display layers. The date parsing logic from the document properties is rewritten according to the specification and now includes unit tests.
Moreover, missing unit tests for the color of a popup annotation have been added.
Finally the styling of the popup is changed slightly to make the text a bit smaller (it's currently quite large in comparison to other viewers) and to make the drop shadow a bit more subtle. The former is done to be able to easily include the modification date in the popup similar to how other viewers do this.