Cloud-CV / EvalAI-ngx

Revamped codebase of EvalAI Frontend
BSD 3-Clause "New" or "Revised" License
33 stars 61 forks source link

Add sidebar to Challenge page #277

Closed jayaike closed 4 years ago

jayaike commented 4 years ago

I have added the sidebar to the challenge page as described in other pages

Screenshots

After Login

Screenshot from 2020-01-23 20-21-36

Before Login

Screenshot from 2020-01-23 20-21-54

GCI Task: https://codein.withgoogle.com/dashboard/task-instances/6659470118092800/

jayaike commented 4 years ago

@Sanji515 @vkartik97 @Ram81 @RishabhJain2018 PTAL :)

Sanji515 commented 4 years ago

@nsjcorps Can you please fix the build? Also by looking at the screenshot it's not looking same as in the current version. There's too much extra space on the top, bottom, left and right of card. And yeah you forgot the background color of sidebar also.

jayaike commented 4 years ago

Sorry about the build @Sanji515 I'm already on it. I used the classes that were in the pages described in the task. You can take a look

jayaike commented 4 years ago

I had not updated my branch. Let me take another screenshot

jayaike commented 4 years ago

I did not make another sidebar. I merely imported it as a component. @Sanji515

The background color has a color...the way it was before.

Please try building on your system so you can take a better look from your end.

codecov-io commented 4 years ago

Codecov Report

Merging #277 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #277   +/-   ##
=======================================
  Coverage   50.64%   50.64%           
=======================================
  Files          66       66           
  Lines        3771     3771           
  Branches      444      444           
=======================================
  Hits         1910     1910           
  Misses       1766     1766           
  Partials       95       95

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 1eb7567...0ac4e80. Read the comment docs.

jayaike commented 4 years ago

I have fixed the build btw @Sanji515

Suryansh5545 commented 4 years ago

@nsjcorp I think these participate, host, hosted and create challenges tab shouldn't appear when user is not logged in?

jayaike commented 4 years ago

I only bypassed the redirect so I could edit the page. That is why it seems that the I am not authenticated @Suryansh5545

Suryansh5545 commented 4 years ago

Oh got it:+1:

Sanji515 commented 4 years ago

@nsjcorps Can you please fix the build? Also by looking at the screenshot it's not looking same as in the current version. There's too much extra space on the top, bottom, left and right of card. And yeah you forgot the background color of sidebar also.

I've checked on my local @nsjcorps , it still doesn't look same as in the current version (the width of challenge container) and please change the background color of sidebar as in the current version.

jayaike commented 4 years ago

@Sanji515 PTAL

Screenshot (134) Screenshot (133)

I also changed it to suit other files.

Sanji515 commented 4 years ago

Screenshot from 2020-01-19 22-52-02

@nsjcorps Can you please fix this(^) ? This is happening on the challenge page only.

jayaike commented 4 years ago

@Sanji515 It looks good on mine as shown in the screenshot. Are you sure you have cloned the updated version?

Sanji515 commented 4 years ago

@Sanji515 It looks good on mine as shown in the screenshot. Are you sure you have cloned the updated version?

Yes, I'm on the updated version, try when user is logged in.

jayaike commented 4 years ago

Is it only on the challenge detail page? or challenge list

Sanji515 commented 4 years ago

Also can we be consistent by removing the sidebar

Is it only on the challenge detail page? or challenge list

Challenge detail page.

jayaike commented 4 years ago

I don't understand

Sanji515 commented 4 years ago

Screenshot from 2020-01-19 22-52-02

It is happening on the challenge overview page when user is logged in.

jayaike commented 4 years ago

I updated it already

jayaike commented 4 years ago

Unfortunately, I am still not seeing that error on challenge page

jayaike commented 4 years ago

How does it look @Sanji515 ?

Sanji515 commented 4 years ago

Lets keep it consistent with the current version, so please remove the sidebar if user is not logged in

jayaike commented 4 years ago

@Sanji515 done

jayaike commented 4 years ago

@Sanji515 PTAL

Sanji515 commented 4 years ago

There's too much extra space on the top, bottom, left and right of card for the resolution between 990px and 1130px. Can you please fix it?

jayaike commented 4 years ago

Do you mean margin or padding? @Sanji515

jayaike commented 4 years ago

I just reduced the margin from the sides @Sanji515 PTAL

jayaike commented 4 years ago

Sorry the push took some time @Sanji515

Sanji515 commented 4 years ago

I just reduced the margin from the sides @Sanji515 PTAL

Or just remove this from here for the small resolutions.

jayaike commented 4 years ago

Done @Sanji515

jayaike commented 4 years ago

Sorry @Sanji515 I edited the wrong div

jayaike commented 4 years ago

Thanks for bearing with me though

jayaike commented 4 years ago

I also had to make the changes to the other files @Sanji515

Sanji515 commented 4 years ago

Hey, still the same. Always please try checking on your local before pushing.

jayaike commented 4 years ago

@Sanji515 I tested it this time. It is centered

jayaike commented 4 years ago

localhost_4200_challenge_2_overview - Google Chrome 1_19_2020 9_42_55 PM

jayaike commented 4 years ago

@Sanji515 PTAL

jayaike commented 4 years ago

That is the only way the &.center css gets called

Sanji515 commented 4 years ago

That is the only way the &.center css gets called

Yes, but the condition gets only applied when the user is not logged in right? Then what is the use of it on the dashboard page and similar other.

jayaike commented 4 years ago

Yes that is true. What do you suggest @Sanji515

jayaike commented 4 years ago

Because it should be tested

Sanji515 commented 4 years ago

Lets remove all those class.center class related changes from the files like analytics, dashboard and similar other.

jayaike commented 4 years ago

How will I center the div then @Sanji515 ?

Sanji515 commented 4 years ago

How will I center the div then @Sanji515

I'm not saying to remove from the challenge.component.html as it is gonna be used as we displaying the page even when user is not authenticated but the pages like dashboard and similar others are not gonna shown if the user is not authenticated so what's the use of class.center="!authService.isAuth". So lets remove the code from these files.

jayaike commented 4 years ago

Okay I get now. They are protected routes so unauthorized users wont see them anyway.

So should I remove them now? @Sanji515

Sanji515 commented 4 years ago

Okay I get now. They are protected routes so unauthorized users wont see them anyway.

So should I remove them now? @Sanji515

yes go ahead!

jayaike commented 4 years ago

Should I also remove from public lists?

Sanji515 commented 4 years ago

Should I also remove from public lists?

nope, not from here.

jayaike commented 4 years ago

Alright. I removed the one's for the protected routes