topcoder-platform / leaderboard-ui

0 stars 3 forks source link

Primary sponsor image adjustment #8

Closed ajefts closed 5 years ago

ajefts commented 5 years ago

We have a field for primary sponsor, which is good. However, it's making the image really big on some pages and impacting the layout. Here is an example:

image

Can we remove the primary sponsor display on all pages EXCEPT the sponsor page? i.e. Leave the logic on this page http://tco-leaderboard-ui-dev.herokuapp.com/page/sponsor/6DACtxIEgwyUMIQACKEIqM, but remove it from the other pages. We'll just show the small icons so all sponsors will get the same treatment.

callmekatootie commented 5 years ago

@ajefts Could we not move that specific sponsor to the other sponsors list and keep the primary sponsor empty for the pages where needed?

ajefts commented 5 years ago

Yes, that’s what I’m suggesting.

On Nov 10, 2018, at 4:16 AM, Mithun Kamath notifications@github.com wrote:

@ajefts Could we not move that specific sponsor to the other sponsors list and keep the primary sponsor empty for the pages where needed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

callmekatootie commented 5 years ago

Sorry, I did not explain it clearly. What I mean is that can't we continue with the current arrangement, where we have a primary sponsor and secondary sponsors instead of removing the primary sponsor from the display altogether? You could leave it empty in contentful but we can keep it's container in the UI - in other words, we don't make any changes to the code. You only ensure that in contentful, primary sponsor is not used

ajefts commented 5 years ago

But then the main sponsor page doesn’t work?

On Nov 11, 2018, at 2:09 AM, Mithun Kamath notifications@github.com wrote:

Sorry, I did not explain it clearly. What I mean is that can't we continue with the current arrangement, where we have a primary sponsor and secondary sponsors instead of removing the primary sponsor from the display altogether? You could leave it empty in contentful but we can keep it's container in the UI - in other words, we don't make any changes to the code. You only ensure that in contentful, primary sponsor is not used

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

callmekatootie commented 5 years ago

Main Sponsor Page: Use the primarySponsor attribute. Provide an image to it and the image will show up All Other pages: Don't use the primarySponsor attribute. Leave it empty. Since there is no primary sponsor to be shown, the UI won't show it.

callmekatootie commented 5 years ago

This way, no code change would be needed

ajefts commented 5 years ago

ah, I think I see what you mean now. Sorry for being slow on this one :)

ajefts commented 5 years ago

actually, 1 issue. The app breaks if I remove the primary sponsor:

image

@callmekatootie

ajefts commented 5 years ago

@callmekatootie Need some help on this one if you're available

callmekatootie commented 5 years ago

huh... checking out why. will get back asap

ajefts commented 5 years ago

I think there is some general error handling missing them components of a page are left empty. I've tried removing things various components from a page, but I always get that same 500 error.

callmekatootie commented 5 years ago

@ajefts Can you remove the primary sponsor and let me know. The error seems to be something else - there is a default value for primary sponsor at the moment, so even if you dont set it, the default image should have been used, but you appear to be getting an error (locally if I try, I get the default image).

Thus I wish to know what's different in your case. Let me know once you are removed the primary sponsor (and url of page which gets affected)

ajefts commented 5 years ago

@callmekatootie I removed the primary. Here is the page: http://scoreboards.topcoder.com/page/finalists/3EvvMhMTEI8gygeIwsgmUy

ajefts commented 5 years ago

Actually, any page that uses the sponsor component errors now. Try any of these....

http://scoreboards.topcoder.com/track/5Rm1zODfk4eCeaoEkkSSq2

bountyC0d3r commented 5 years ago

@ajefts issue has been fixed, merged and deployed. Can you please verify and close the ticket?