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

Move test annotations to edit phase section #3570

Closed
wants to merge 8 commits into from
Closed

Move test annotations to edit phase section #3570

wants to merge 8 commits into from

Conversation

gautamjajoo
Copy link
Member

@gautamjajoo gautamjajoo commented Aug 14, 2021 •
edited
Loading

  • Move test annotations from edit-phase-modal to edit phase section
  • Add note for file size>100MB
    @Ram81 @Kajol-Kumari

Screenshot from 2021-08-14 23-42-03

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2021 •
edited
Loading

Codecov Report

Merging #3570 (6d2d3db) into master (96968d6) will decrease coverage by 1.42%.
The diff coverage is 36.94%.

@@            Coverage Diff             @@
##           master    #3570      +/-   ##
==========================================
- Coverage   72.93%   71.50%   -1.43%     
==========================================
  Files          83       20      -63     
  Lines        5368     3239    -2129     
==========================================
- Hits         3915     2316    -1599     
+ Misses       1453      923     -530
Impacted Files Coverage Δ
frontend/src/js/controllers/authCtrl.js 53.91% <6.38%> (-12.95%) ⬇️
frontend/src/js/controllers/profileCtrl.js 79.76% <20.00%> (-13.10%) ⬇️
frontend/src/js/controllers/permissionCtrl.js 36.36% <22.22%> (-63.64%) ⬇️
frontend/src/js/controllers/challengeCtrl.js 63.51% <36.71%> (-10.19%) ⬇️
frontend/src/js/controllers/updateProfileCtrl.js 82.55% <44.44%> (-10.30%) ⬇️
frontend/src/js/controllers/challengeListCtrl.js 95.74% <50.00%> (+1.06%) ⬆️
...ntend/src/js/controllers/challengeHostTeamsCtrl.js 70.50% <66.66%> (-1.18%) ⬇️
frontend/src/js/controllers/teamsCtrl.js 71.17% <75.00%> (ø)
frontend/src/js/controllers/ChallengeInviteCtrl.js 100.00% <100.00%> (ø)
frontend/src/js/controllers/SubmissionFilesCtrl.js 95.45% <100.00%> (ø)
... and 30 more
Impacted Files Coverage Δ
frontend/src/js/controllers/authCtrl.js 53.91% <6.38%> (-12.95%) ⬇️
frontend/src/js/controllers/profileCtrl.js 79.76% <20.00%> (-13.10%) ⬇️
frontend/src/js/controllers/permissionCtrl.js 36.36% <22.22%> (-63.64%) ⬇️
frontend/src/js/controllers/challengeCtrl.js 63.51% <36.71%> (-10.19%) ⬇️
frontend/src/js/controllers/updateProfileCtrl.js 82.55% <44.44%> (-10.30%) ⬇️
frontend/src/js/controllers/challengeListCtrl.js 95.74% <50.00%> (+1.06%) ⬆️
...ntend/src/js/controllers/challengeHostTeamsCtrl.js 70.50% <66.66%> (-1.18%) ⬇️
frontend/src/js/controllers/teamsCtrl.js 71.17% <75.00%> (ø)
frontend/src/js/controllers/ChallengeInviteCtrl.js 100.00% <100.00%> (ø)
frontend/src/js/controllers/SubmissionFilesCtrl.js 95.45% <100.00%> (ø)
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9b05b5...6d2d3db. Read the comment docs.

@gautamjajoo
Copy link
Member Author

@Ram81 @Kajol-Kumari
Screenshot from 2021-08-21 14-56-27
Screenshot from 2021-08-21 14-56-17

Kajol-Kumari
Kajol-Kumari suggested changes Aug 23, 2021
View reviewed changes
Copy link
Member

@Kajol-Kumari Kajol-Kumari left a comment

Choose a reason for hiding this comment

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

@gautamjajoo minor change required

frontend_v2/src/app/components/challenge/challengesettings/challengesettings.component.html Outdated
<div class="fw-light">
<p class="fs-16">
<strong> Note: </strong> For test annotations > 100MB, please use
<a href=" https://cli.eval.ai/#challenge-annotations"><strong class="blue-text"> evalai-cli </strong></a> to update the test annotations.
Copy link
Member

Choose a reason for hiding this comment

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

Can we please use target="_blank" here so that the annotations page open up in new tab?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@Kajol-Kumari Kajol-Kumari mentioned this pull request Aug 23, 2021
Open
Ram81
Ram81 requested changes Aug 26, 2021
View reviewed changes
frontend_v2/src/app/components/challenge/challengesettings/challengesettings.component.html Outdated
<div class="col-md-11 col-sm-11 col-xs-10" *ngIf="isChallengeHost">
<div class="fw-light">
<p class="fs-16">
<strong> Note: </strong> For test annotations > 100MB, please use
Copy link
Member

Choose a reason for hiding this comment

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

@gautamjajoo the note should appear at the end of Edit phase section. It should be show below the Test annotaitons upload option

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

frontend_v2/src/app/components/challenge/challengesettings/challengesettings.component.html Outdated
<strong class="text-light-black">Test Annotations </strong>
</span>
<a class="pointer fs-15" (click)="editTestAnnotations()">
<i class="fa fa-file" aria-hidden="true"> </i>
Copy link
Member

Choose a reason for hiding this comment

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

Please use the upload icon for test annotations

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

frontend_v2/src/app/components/challenge/challengesettings/challengesettings.component.ts Outdated
* Parameters of the modal
*/
const PARAMS = {
title: 'Cli Instructions for updating test annotations',
Copy link
Member

Choose a reason for hiding this comment

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

Title should be Instructions

@gautamjajoo
Copy link
Member Author

Closing this PR since unnecessary changes have been made. Will open a new PR.

@gautamjajoo gautamjajoo mentioned this pull request Aug 30, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kajol-Kumari Kajol-Kumari Kajol-Kumari requested changes

@Ram81 Ram81 Ram81 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gautamjajoo @codecov-commenter @Ram81 @Kajol-Kumari