Skip to content

fix: line label invisible in dark theme#1465

Merged
martinRenou merged 3 commits intobqplot:masterfrom
mariobuikhuizen:fix_line_label_invisible_in_dark_theme
Feb 9, 2022
Merged

fix: line label invisible in dark theme#1465
martinRenou merged 3 commits intobqplot:masterfrom
mariobuikhuizen:fix_line_label_invisible_in_dark_theme

Conversation

@mariobuikhuizen
Copy link
Copy Markdown
Contributor

@mariobuikhuizen mariobuikhuizen commented Feb 9, 2022 •
edited
Loading

To reproduce:

from bqplot import *


x = LinearScale()
y = LinearScale()

line = Lines(x=[0], 
             y=[0],
             scales={'x': x, 'y': y},
             labels=['My label'],
             labels_visibility='label')
ax_x = Axis(scale=x, label='X')
ax_y = Axis(scale=y, label='Y', orientation='vertical')

Figure(marks=[line], axes=[ax_x, ax_y])
Light Dark
Screenshot 2022-02-09 at 14 51 25 Screenshot 2022-02-09 at 14 51 39

With this PR:

Screenshot 2022-02-09 at 15 19 58

mariobuikhuizen added a commit to mariobuikhuizen/jdaviz that referenced this pull request Feb 9, 2022
@mariobuikhuizen
fix: workaround for invisible line label in dark theme
afd5429 
Can be removed when bqplot/bqplot#1465 is
released.
mariobuikhuizen added a commit to mariobuikhuizen/jdaviz that referenced this pull request Feb 9, 2022
@mariobuikhuizen
fix: workaround for invisible line label in dark theme
5231f97 
Can be removed when bqplot/bqplot#1465 is
released.
@mariobuikhuizen mariobuikhuizen mentioned this pull request Feb 9, 2022
Merged
9 tasks
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022

Thanks!

@maartenbreddels
Copy link
Copy Markdown
Member

maartenbreddels commented Feb 9, 2022

@martinRenou do you think it's worth adding a visual test? I'm ok with the PR as it is.

@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022 •
edited
Loading

I was about to ask adding a small visual test :) You can probably add a cell that adds a label in https://github.com/bqplot/bqplot/blob/master/ui-tests/tests/notebooks/scatter_update.ipynb. After that you can trigger the bot to update the test references by commenting "update galata references" in the PR.

@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022

Let's backport this as well once it's merged

@mariobuikhuizen
Copy link
Copy Markdown
Contributor Author

mariobuikhuizen commented Feb 9, 2022

update galata references

@maartenbreddels
Copy link
Copy Markdown
Member

maartenbreddels commented Feb 9, 2022

@martinRenou is it normal that CI doesn't run after the update of galata? so can we now merge this?

@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022

Yeah commits generated from the Github bot are not triggering the CI (I read in the Github docs that this was to prevent an infinite loop of triggering CI, there might be a way to enable it). Closing/reopening to trigger it for now.

@martinRenou martinRenou closed this Feb 9, 2022
@martinRenou martinRenou reopened this Feb 9, 2022
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022

meeseeksdev please backport to 0.12.x

@lumberbot-app
Copy link
Copy Markdown

lumberbot-app bot commented Feb 9, 2022

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 0.12.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -m1 78bf294ad1c33f1df7fd8ecafae40f93da28f2bc
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #1465: fix: line label invisible in dark theme'
  1. Push to a named branch:
git push YOURFORK 0.12.x:auto-backport-of-pr-1465-on-0.12.x
  1. Create a PR against branch 0.12.x, I would have named this PR:

"Backport PR #1465 on branch 0.12.x (fix: line label invisible in dark theme)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@martinRenou martinRenou merged commit 71e7cd8 into bqplot:master Feb 9, 2022
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 9, 2022

Thanks!

martinRenou pushed a commit to martinRenou/bqplot that referenced this pull request Feb 10, 2022
@mariobuikhuizen @martinRenou
Backport PR bqplot#1465: fix: line label invisible in dark theme
dc41da3
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 10, 2022

meeseeksdev please backport to 0.12.x

@lumberbot-app
Copy link
Copy Markdown

lumberbot-app bot commented Feb 10, 2022

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 0.12.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -m1 71e7cd86746b8ec7674a02e2816cdd3c8364ac19
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #1465: fix: line label invisible in dark theme'
  1. Push to a named branch:
git push YOURFORK 0.12.x:auto-backport-of-pr-1465-on-0.12.x
  1. Create a PR against branch 0.12.x, I would have named this PR:

"Backport PR #1465 on branch 0.12.x (fix: line label invisible in dark theme)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 10, 2022

meeseeksdev please backport to 0.12.x

meeseeksmachine pushed a commit to meeseeksmachine/bqplot that referenced this pull request Feb 10, 2022
@martinRenou @meeseeksmachine
Backport PR bqplot#1465: fix: line label invisible in dark theme
b68b00e
@meeseeksmachine meeseeksmachine mentioned this pull request Feb 10, 2022
Merged
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 10, 2022 •
edited
Loading

We'll never get replaced by machines for sure 😄 Sorry for the noise on this PR.

martinRenou added a commit that referenced this pull request Feb 10, 2022
@martinRenou
Merge pull request #1472 from meeseeksmachine/auto-backport-of-pr-146…
ac74eff 
…5-on-0.12.x

Backport PR #1465 on branch 0.12.x (fix: line label invisible in dark theme)
kecnry pushed a commit to kecnry/jdaviz that referenced this pull request Feb 10, 2022
@mariobuikhuizen @kecnry
fix: workaround for invisible line label in dark theme
e8ca844 
Can be removed when bqplot/bqplot#1465 is
released.
kecnry pushed a commit to kecnry/jdaviz that referenced this pull request Feb 10, 2022
@mariobuikhuizen @kecnry
fix: workaround for invisible line label in dark theme
497aff1 
Can be removed when bqplot/bqplot#1465 is
released.
@pllim
Copy link
Copy Markdown
Contributor

pllim commented Feb 10, 2022

Hi. What release will this patch be in and when is the release expected to come out? Thanks!

@pllim pllim mentioned this pull request Feb 10, 2022
Merged
25 tasks
@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 10, 2022

Hey :) It will be included in the next 0.12.x release. I will do it soonish.

@pllim
Copy link
Copy Markdown
Contributor

pllim commented Feb 10, 2022

Please let me know the value of x when that happens so we can properly pin it downstream. Thank you! 🙏

@martinRenou
Copy link
Copy Markdown
Member

martinRenou commented Feb 11, 2022

@pllim bqplot 0.12.33 is out :)

pllim added a commit to pllim/jdaviz that referenced this pull request Feb 14, 2022
@pllim
MNT: Bump bqplot minversion to 0.12.33
c6d5595 
to get bqplot/bqplot#1465
@pllim pllim mentioned this pull request Feb 14, 2022
Merged
9 tasks
@pllim
Copy link
Copy Markdown
Contributor

pllim commented Feb 14, 2022

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mariobuikhuizen @martinRenou @maartenbreddels @pllim