kedro icon indicating copy to clipboard operation
kedro copied to clipboard

Get rid of GitHub magic in README [Quick Win]

Open astrojuanlu opened this issue 1 year ago • 8 comments

@marrrcin reported that he saw the README like this

image

and it's because of a bad combination of the magic we're using to detect light/dark theme

https://github.com/kedro-org/kedro/blob/8257d55ec96abbc1ce18ae229da8c773c60bfc1b/README.md?plain=1#L1-L2

and how GitHub assumes night = dark

image

This has created other problems in the past https://github.com/kedro-org/kedro/pull/2961#issuecomment-1688542270

Given that this is broken on PyPI and half-broken on GitHub, I'd say let's get rid of this.

A strawman proposal would be something like

image

But better to have this properly designed.

astrojuanlu avatar Dec 12 '23 15:12 astrojuanlu

Moving this to Viz as it'll be part of design initially and quite likely require web savvy too.

stichbury avatar Jan 12 '24 09:01 stichbury

@SajidAlamQB also reported this issue on pypi - https://pypi.org/project/kedro/

rashidakanchwala avatar Feb 26 '24 15:02 rashidakanchwala

We don't need tech design for this. We need to just do it.

astrojuanlu avatar Feb 26 '24 16:02 astrojuanlu

I encountered this when publishing github action. The way we do conditional images is deprecated. This link should help.

ravi-kumar-pilla avatar Mar 25 '24 15:03 ravi-kumar-pilla

@ravi-kumar-pilla could you check if the <picture> thing is rendered correctly on PyPI?

astrojuanlu avatar Mar 25 '24 15:03 astrojuanlu

@ravi-kumar-pilla could you check if the <picture> thing is rendered correctly on PyPI?

@astrojuanlu I see an open ticket - https://github.com/pypi/warehouse/issues/11251 . Needs some more searching/testing when we work on the ticket.

I tried testing the html and picture tag seems to work fine -

image

image

ravi-kumar-pilla avatar Mar 25 '24 16:03 ravi-kumar-pilla

The way to test this would be to upload a test version to https://test.pypi.org/project/kedro/ or a newly registered package only for testing purposes.

astrojuanlu avatar Mar 25 '24 16:03 astrojuanlu

We tried to replicate this issue and it doesn't exist anymore for Readme anymore. It does exist for Pypi which we will work on fixing.

rashidakanchwala avatar Aug 12 '24 15:08 rashidakanchwala