pharmaverse / admiral

ADaM in R Asset Library
https://pharmaverse.github.io/admiral
Apache License 2.0
224 stars 63 forks source link

Website Package version number is illegible due to the text colour #2360

Closed rossfarrugia closed 5 months ago

rossfarrugia commented 8 months ago

Background Information

image

Definition of Done

No response

bms63 commented 8 months ago

I can't remember...was it ever readable...I feel like it was

rossfarrugia commented 8 months ago

I can't remember...was it ever readable...I feel like it was

yep, i had an old screenshot in some training slides and it used to be.

manciniedoardo commented 8 months ago

I believe it just happened when going from 1.0.1 to 1.0.2?

rossfarrugia commented 8 months ago

not sure sorry!

Fanny-Gautier commented 7 months ago

It must be default colour, but it seems feasible to change it. This could be defined within the development section of the _pkgdown.yml file. image

rossfarrugia commented 7 months ago

@Fanny-Gautier this seems a good article: https://pkgdown.r-lib.org/articles/customise.html. Would you be interested in self-assigning this one and having an explore to see if you can get it looking cleaner?

Fanny-Gautier commented 7 months ago

Here are the different colors that we can get with the following options (mode: automatic):

For the release, I guess that the mode has been updated to "release" ? In that case the version gets the default color. image I am trying to see the default colors of the "bootswatch: flatly" theme, and if we can change them without having to customize the entire theme.

rossfarrugia commented 7 months ago

thanks Fanny! the default you show there was how it used to be and that light grey colour was fine but for some reason it now went to dark grey which conflicts with the background. light grey or white would be best as the other colours look confusing, like the red one would make me assume the package is not ready.

Fanny-Gautier commented 7 months ago

I am well getting the version number in black with the release mode with the below setting:

We can force it with the light grey color by setting the version_label to "default" as per below statements:

Could you please confirm that I should force it to "default" in the _pkgdown.yml file, whatever the mode (auto/release) ? It means that the version number does not appear in red anymore when we work on development branches.

Current _pkgdown.yml file:

manciniedoardo commented 7 months ago

Otherwise it needs to be updated for the official release only, when the mode is set to release.

Ideally, it would be red for the dev site, and then grey/white for the main site (this is how it was before)

bms63 commented 7 months ago

@ddsjoberg, @cicdguy @dgrassellyb - can you all weigh in here. something happened since 1.0.0 to 1.0.1/2 where our package version is hard to read now.

Fanny-Gautier commented 7 months ago

Ideally, it would be red for the dev site, and then grey/white for the main site (this is how it was before)

Is _pkgdown.yml file updated and the current mode updated from "auto" to "release" for the official release ? What is the process for the official releases ?

bms63 commented 7 months ago

Here is some rough guidance on how we do it. https://pharmaverse.github.io/admiraldev/dev/articles/release_strategy.html

You would need to inspect the CI action for pkgdown as well in the admiralci repo

Fanny-Gautier commented 7 months ago

@cicdguy, could you please check if this issue get resolved while fixing the "404 errors - previous versions of admiral website" ? If not I'll investigate a bit more. Thank you.

cicdguy commented 7 months ago

@Fanny-Gautier - as indicated in yesterday's call, it's on track for resolution following the implementation of the issues mentioned here, reverting the workflows to when the multiversion pkgdown docs were working (before then introduction of the custom solution that @ddsjoberg has implemented), and restoring the previous versions from the git history.

cicdguy commented 7 months ago

Oh sorry I misunderstood @Fanny-Gautier - I don't believe fixing the missing versions issue will actually fix the underlying issue we're witnessing here (discoloration of the version). That will in fact be fixed by what you already mentioned eariler:

  1. Either changing the underlying theme for the docs
  2. Setting version_label: default in the pkgdown.yml
bms63 commented 7 months ago

I guess this is confusing as the version number used to be a different color and has only recently changed to be unreadable. can we hold off on this @Fanny-Gautier until multi-versions is re-implemented please.

ddsjoberg commented 7 months ago

reverting the workflows to when the multiversion pkgdown docs were working (before then introduction of the custom solution that @ddsjoberg has implemented), and restoring the previous versions from the git history.

@cicdguy to be clear, all updates to admiral workflows were done by the IDR team. I added an article to the pkgdown site that links to the websites.

cicdguy commented 7 months ago

@ddsjoberg - okay let's take down this article then. It's been a source of confusion since its inception.

ddsjoberg commented 7 months ago

there's no need to modify the plans we made in our meeting yesterday

cicdguy commented 7 months ago

Indeed. No intention of modifying it - it's still part of the plan. We'll be removing the article once the new features have been implemented, given that it'll be part of the reversion to the old state.

Fanny-Gautier commented 6 months ago

Is there any update for this topic or is still on hold ? Thanks.

bms63 commented 6 months ago

@cicdguy said there is progress on it and should be available mid-Mayish. Thanks for checking!!

cicdguy commented 5 months ago

I've restored the previous pages. Will be updating the workflows soon.

bms63 commented 5 months ago

Thanks @cicdguy !!

I noticed in the dev mode of the site that the dropdowns have turned dark. Not sure if was due to the update?
image

cicdguy commented 5 months ago

I think you probably have cached or stale CSS in your browser. Looks fine to me:

Screenshot 2024-05-14 at 7 56 32 AM
cicdguy commented 5 months ago

OR it could be the default in the dev versions? Not sure, but it's not due to the restoration.

cicdguy commented 5 months ago

So @manciniedoardo and I looked into the dark background issue and it looks like the flatly theme uses the gray background by default: https://github.com/thomaspark/bootswatch/blame/v5.3.1/dist/flatly/bootstrap.css#L3705. Color ref: https://www.colorhexa.com/343a40 You can cross-reference or modify the CSS here: https://github.com/pharmaverse/admiral/blob/gh-pages/dev/deps/bootstrap-5.3.1/bootstrap.min.css

bms63 commented 5 months ago

Thanks all - I've been paying attention to color stuff more since the version number went dark on us 6 months ago. I couldn't remember if this dark menu had always been present or not. Thanks for checking.

Fanny-Gautier commented 5 months ago

I also see it with light color in dev mode: image

Fanny-Gautier commented 5 months ago

I am working on it today. But I find difficult to understand which --bs- variable is used in the CSS file when the version_label is NULL / Not defined in _pkgdown.yml. For now it seems that data-bs-theme="light" applies for development: mode: auto and development: mode: release. But I don't find the reason why it is --bs-danger which applies for version_label: NULL/Not defined for development: mode: auto and I don't know which --bs- variable applies for version_label: NULL/Not defined for development: mode: release

bms63 commented 5 months ago

I am working on it today. But I find difficult to understand which --bs- variable is used in the CSS file when the version_label is NULL / Not defined in _pkgdown.yml. For now it seems that data-bs-theme="light" applies for development: mode: auto and development: mode: release. But I don't find the reason why it is --bs-danger which applies for version_label: NULL/Not defined for development: mode: auto and I don't know which --bs- variable applies for version_label: NULL/Not defined for development: mode: release

@cicdguy should we be going down this path? Is it possible for the CI action for the pkgdown build to take care of this type of thing for us? I don't remember us messing with the stuff when the pkg version was a white color so unclear when or why this dark color started to appear.

cicdguy commented 5 months ago

Yeah I have no idea. I'm not a front end expert so my understanding of the issue is very limited 😢

ddsjoberg commented 5 months ago

I think this is coming from this line in the CSS with text-muted

<small class="nav-text text-muted me-auto" data-bs-toggle="tooltip" data-bs-placement="bottom" title="Released version">1.0.2</small>

The tricky part will be overriding the default. A couple of things I would try (and apologies if this is already suggested above, it didn't read the whole thing 🤷🏼 )

  1. Modifying the text-muted tag in the _pkgdown.yml under the bslib section.
  2. We can inject custom CSS into our site following the instructions here: https://pkgdown.r-lib.org/articles/customise.html#additional-html-and-files . I am way out of my depth here, but maybe we could redefine text-muted? IDK!
Fanny-Gautier commented 5 months ago

I think this is coming from this line in the CSS with text-muted

<small class="nav-text text-muted me-auto" data-bs-toggle="tooltip" data-bs-placement="bottom" title="Released version">1.0.2

Yesterday, I saw a previous post from you on a forum and tried to search for that line, but I could not find it in the CSS file. Did I look at the wrong place?

Fanny-Gautier commented 5 months ago

The easiest solution that I have for now it to manually force it as suggested a few times ago in _pkdown.yml as following and at releases time: image image

And set it back to danger color (red) after the release and for the auto mode as it currently is set: image image

bms63 commented 5 months ago

Shall we just give it a try now?

Fanny-Gautier commented 5 months ago

Shall we just give it a try now?

I have created the feature branch and pushed the updates. Shall I create a PR or is it fine for you to try from the corresponding feature branch?

bms63 commented 5 months ago

Could you branch off the latest release? I wanted to re-release admiral to try out the fix, but not have any of the new stuff visible.

https://stackoverflow.com/questions/10940981/how-to-create-a-new-branch-from-a-tag

Fanny-Gautier commented 5 months ago

Ok, I'll create another branch from tag v1.0.2. thanks. Did you realize that the color is also different when not on the index/main page ? image

bms63 commented 5 months ago

The plot thickens!

Fanny-Gautier commented 5 months ago

@bms63 I have created 2360_website_package_version_number_color_tag_v1-0-2 as per your suggestion. Testing it before you can proceed with an official test, I am facing a funny thing... 🙃 When the following is run: image I well get the number 1.0.2.9012 in red color image But if I click on Reference page, it switches to 1.0.2.9035 in green 😵 image

I thought that it was due to the other statements commented out (#mode: release + #version_label: success) so I deleted them, but it does not matter. still switching to green now on other pages than the index one 😵😵😵

bms63 commented 5 months ago

@bms63 I have created 2360_website_package_version_number_color_tag_v1-0-2 as per your suggestion. Testing it before you can proceed with an official test, I am facing a funny thing... 🙃 When the following is run: image I well get the number 1.0.2.9012 in red color image But if I click on Reference page, it switches to 1.0.2.9035 in green 😵 image

I thought that it was due to the other statements commented out (#mode: release + #version_label: success) so I deleted them, but it does not matter. still switching to green now on other pages than the index one 😵😵😵

that is interesting behavior...but i don't really mind it if the version label goes back to white.

Fanny-Gautier commented 5 months ago

I have restarted R, and will run build_site() instead of build_articles()... I keep you in touch once it's ready for you to test an official release.

Fanny-Gautier commented 5 months ago

I am getting some errors with build_site(). I have to go now, sorry, I'll continue on monday image image

Fanny-Gautier commented 5 months ago

Hi @bms63 , It works well on the main branch (green for all pages with version_label: success, and red for all pages with version_label: danger), but on the branch _2360_website_package_version_number_color_tagv1-0-2, I am unable to build the site. I get the following error message: image

Lina2689 commented 5 months ago

@Fanny-Gautier @bms63 It's running fine for me on branch 2360_website_package_version_number_color_tag_v1-0-2.

Fanny-Gautier commented 5 months ago

@bms63 Is it possible that I face the errors due to the admiraldev package version ? Lina is having 1.0.0.9011 while I have 1.0.0.9027. image

It seems that my error comes from an update in admiraldev between version 1.0.0.9016 and 1.0.0.9017. image Is there a way to load again version 1.0.0.9011 ?

thanks

Fanny-Gautier commented 5 months ago

Hi @bms63, I tried again while installing {admiraldev} v1.0.0 and it works fine again. You can now try the official release from the branch _2360_website_package_version_number_color_tagv1-0-2. Thank you.

Thanks @Lina2689 for all the tests done from your end ;-)

bms63 commented 5 months ago

Hi @bms63, I tried again while installing {admiraldev} v1.0.0 and it works fine again. You can now try the official release from the branch _2360_website_package_version_number_color_tagv1-0-2. Thank you.

Thanks @Lina2689 for all the tests done from your end ;-)

Okay! It finally made sense to me why we needed to install v1.0.0 for admiraldev...this is admiral 1.0.2

It looks fine to me. Can we change it to the light gray and just merge to main from a non-tagged branch. I'm not going to have time to test it out with a mini-release...will just do it live!! :)