Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rossfarrugia opened this issue Mar 5, 2024 · 50 comments · Fixed by #2451
Closed

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

rossfarrugia opened this issue Mar 5, 2024 · 50 comments · Fixed by #2451
Assignees

Comments

@rossfarrugia
Copy link
Collaborator

Background Information

image

Definition of Done

No response

@bms63
Copy link
Collaborator

bms63 commented Mar 5, 2024

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

@rossfarrugia
Copy link
Collaborator Author

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
Copy link
Collaborator

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

@rossfarrugia
Copy link
Collaborator Author

not sure sorry!

@Fanny-Gautier
Copy link
Collaborator

Fanny-Gautier commented Mar 15, 2024

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
Copy link
Collaborator Author

@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 Fanny-Gautier self-assigned this Mar 15, 2024
@Fanny-Gautier
Copy link
Collaborator

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

  • development:
    • mode: auto
    • version_label: default
      image
    • version_label: info
      image
    • version_label: warning
      image
    • version_label: danger
      image

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
Copy link
Collaborator Author

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
Copy link
Collaborator

Fanny-Gautier commented Mar 15, 2024

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

  • development:
    • mode: release
    • no version_label defined:
      image

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

  • development:
    • mode: release
    • version_label: default
      image

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.

  • development:
    • mode: auto
    • version_label: default
      image

Current _pkgdown.yml file:

  • development:
    • mode: auto
    • no version_label defined:
      image

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

@manciniedoardo
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented Mar 15, 2024

@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
Copy link
Collaborator

Fanny-Gautier commented Mar 15, 2024

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
Copy link
Collaborator

bms63 commented Mar 15, 2024

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
Copy link
Collaborator

@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
Copy link
Collaborator

cicdguy commented Mar 21, 2024

@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
Copy link
Collaborator

cicdguy commented Mar 21, 2024

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
Copy link
Collaborator

bms63 commented Mar 21, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

cicdguy commented Mar 21, 2024

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

@ddsjoberg
Copy link
Collaborator

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

@cicdguy
Copy link
Collaborator

cicdguy commented Mar 22, 2024

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
Copy link
Collaborator

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

@bms63
Copy link
Collaborator

bms63 commented May 3, 2024

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

@cicdguy
Copy link
Collaborator

cicdguy commented May 14, 2024

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

@bms63
Copy link
Collaborator

bms63 commented May 14, 2024

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
Copy link
Collaborator

cicdguy commented May 14, 2024

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
Copy link
Collaborator

cicdguy commented May 14, 2024

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

@cicdguy
Copy link
Collaborator

cicdguy commented May 14, 2024

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
Copy link
Collaborator

bms63 commented May 14, 2024

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
Copy link
Collaborator

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

@Fanny-Gautier
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented May 23, 2024

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
Copy link
Collaborator

cicdguy commented May 23, 2024

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

@ddsjoberg
Copy link
Collaborator

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
Copy link
Collaborator

Fanny-Gautier commented May 24, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented May 24, 2024

Shall we just give it a try now?

Fanny-Gautier added a commit that referenced this issue May 24, 2024
… mode, and keep red (danger) for auto/development mode
@Fanny-Gautier
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented May 24, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented May 24, 2024

The plot thickens!

Fanny-Gautier added a commit that referenced this issue May 24, 2024
@Fanny-Gautier
Copy link
Collaborator

@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
Copy link
Collaborator

bms63 commented May 24, 2024

@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
Copy link
Collaborator

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
Copy link
Collaborator

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

@Fanny-Gautier
Copy link
Collaborator

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_tag_v1-0-2, I am unable to build the site. I get the following error message:
image

@Lina2689
Copy link
Contributor

Lina2689 commented May 27, 2024

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

@Fanny-Gautier
Copy link
Collaborator

Fanny-Gautier commented May 27, 2024

@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
Copy link
Collaborator

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_tag_v1-0-2. Thank you.

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

@bms63
Copy link
Collaborator

bms63 commented May 30, 2024

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_tag_v1-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!! :)

@Fanny-Gautier Fanny-Gautier linked a pull request May 30, 2024 that will close this issue
15 tasks
bms63 pushed a commit that referenced this issue May 30, 2024
* #2360 Set version number package color to green (success) for release mode, and keep red (danger) for auto/development mode

* #2360 force version_label to "danger" for auto mode and to "default" for release mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

7 participants