Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Modify the challenge overview page #142

Merged
merged 25 commits into from
Jul 3, 2019

Conversation

Sanji515
Copy link
Member

@Sanji515 Sanji515 commented Jun 10, 2019

Changes proposed in this pull request:

  • Changed some of the minor styling from the current version like border-radius of the card from 4px to 10px and similar other things also

  • Link to live demo: http://pr-142-evalai.surge.sh

Note: Edit challenge description is not in this PR, will open another PR for that

Screenshot:

Screenshot from 2019-06-10 16-58-36

Changed some of the minor styling from the current version
Note: Edit description feature is not in this PR.
Sanji515 added a commit to Sanji515/EvalAI-ngx that referenced this pull request Jun 12, 2019
This PR is an extension of PR(Cloud-CV#142).
Added feature to edit the challenge description using the `froala editor`.
@Sanji515
Copy link
Member Author

@lunayach
Copy link
Member

lunayach commented Jun 12, 2019

@Sanji515 I didn't see unit tests with this PR. Do you plan to add them? (Also, I see the current build failing.)

@Sanji515
Copy link
Member Author

Sanji515 commented Jun 12, 2019

@lunayach well I'm thinking to open new PR for the tests, is there anything wrong in it?

@lunayach
Copy link
Member

@Sanji515 It's okay, but ideally, each PR should be accompanied with the corresponding unit tests added for the added/modified code. Isn't that the purpose of unit tests? Checking (though only to some extent) working of the added code.

Sanji515 added a commit to Sanji515/EvalAI-ngx that referenced this pull request Jun 12, 2019
- Modified the challenge evaluation page and added the feature to edit the text and evaluation script file.
- This is an extension of PR (Cloud-CV#142 & Cloud-CV#144)
@codecov-io
Copy link

codecov-io commented Jun 21, 2019

Codecov Report

Merging #142 into master will decrease coverage by 1%.
The diff coverage is 31.93%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   54.22%   53.22%   -1.01%     
==========================================
  Files          55       55              
  Lines        2353     2463     +110     
  Branches      253      261       +8     
==========================================
+ Hits         1276     1311      +35     
- Misses       1002     1077      +75     
  Partials       75       75
Impacted Files Coverage Δ
...lengesubmissions/challengesubmissions.component.ts 35.55% <0%> (ø) ⬆️
src/app/components/profile/profile.component.ts 51.25% <0%> (ø) ⬆️
...ponents/publiclists/teamlist/teamlist.component.ts 35.22% <0%> (ø) ⬆️
...rc/app/components/utility/input/input.component.ts 59.49% <100%> (+0.51%) ⬆️
...rc/app/components/challenge/challenge.component.ts 41.37% <22.8%> (-36.05%) ⬇️
...rc/app/components/utility/modal/modal.component.ts 42.59% <38.46%> (-2.65%) ⬇️
src/app/services/challenge.service.ts 32.07% <38.88%> (+0.86%) ⬆️
src/app/services/endpoints.service.ts 65.15% <50%> (-0.98%) ⬇️
...e/challengeoverview/challengeoverview.component.ts 65.51% <50%> (-34.49%) ⬇️
Impacted Files Coverage Δ
...lengesubmissions/challengesubmissions.component.ts 35.55% <0%> (ø) ⬆️
src/app/components/profile/profile.component.ts 51.25% <0%> (ø) ⬆️
...ponents/publiclists/teamlist/teamlist.component.ts 35.22% <0%> (ø) ⬆️
...rc/app/components/utility/input/input.component.ts 59.49% <100%> (+0.51%) ⬆️
...rc/app/components/challenge/challenge.component.ts 41.37% <22.8%> (-36.05%) ⬇️
...rc/app/components/utility/modal/modal.component.ts 42.59% <38.46%> (-2.65%) ⬇️
src/app/services/challenge.service.ts 32.07% <38.88%> (+0.86%) ⬆️
src/app/services/endpoints.service.ts 65.15% <50%> (-0.98%) ⬇️
...e/challengeoverview/challengeoverview.component.ts 65.51% <50%> (-34.49%) ⬇️

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 965800b...d06b1db. Read the comment docs.

@Sanji515
Copy link
Member Author

Copy link
Member

@Shekharrajak Shekharrajak left a comment

Choose a reason for hiding this comment

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

Can you try to write some testcases for your newly(you can try for the old one as well) created method otherwise later on no one will check all the UI if they are breaking any existing feature or functionality.

src/app/components/challenge/challenge.component.html Outdated Show resolved Hide resolved
src/app/components/challenge/challenge.component.html Outdated Show resolved Hide resolved
@Sanji515
Copy link
Member Author

Sanji515 commented Jun 24, 2019

Can you try to write some testcases for your newly

@Shekharrajak As other PR's dependent on each other so without merging you cannot review the functionality of other PR's, so thinking to open separate PR's for the tests as they are merged into master

@Shekharrajak
Copy link
Member

Okay! You can open a WIP PR titled 'Specs of challenge overview page #142 ', because I don't want to forget it because it is common that developers write code and after new release old functionalities break, because of improper maintenance and specs.

@Sanji515
Copy link
Member Author

@Shekharrajak I've applied the changes, please review

@Sanji515
Copy link
Member Author

@Shekharrajak Please review

@Sanji515
Copy link
Member Author

Hey @Shekharrajak I've formatted the code, please review if it looks good to you

Instead of const made the apiCall a component variable so that it will be easy to write the test for the API call
@Sanji515
Copy link
Member Author

@Shekharrajak Please review

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

Successfully merging this pull request may close these issues.

5 participants