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

Fix welcome notification on wk.org #7623

Merged
merged 4 commits into from
Feb 15, 2024
Merged

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Feb 13, 2024

This PR is a follow up for the antd v5 upgrade and fixes the styling of the "Welcome"/"Sign up for free" notification on wk.org.

Before:
image

After:
image

Steps to test:

  • Copy the URI/link to a dataset for them the toolbar
  • Open the link in an incognito browser window
  • The welcome notification should show

Issues:


(Please delete unneeded items, merge only when none are left open)

@hotzenklotz hotzenklotz self-assigned this Feb 13, 2024
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is ready for review? Code looks good, but one thing:

The learn more button is a bit hard to read when it's hovered (independent of dark / light mode):

image

@hotzenklotz
Copy link
Member Author

I assume this is ready for review? Code looks good, but one thing:

Yes, it is ready.

The learn more button is a bit hard to read when it's hovered (independent of dark / light mode):

image

Not really sure what to do about this. It is an antd ghost button and that is the default behavior: https://ant.design/components/button?theme=dark#components-button-demo-ghost

@philippotto
Copy link
Member

Not really sure what to do about this.

Can you slap on a class which changes the hovering color? Since the bg is always blue, it should be independent of the theme.

@hotzenklotz
Copy link
Member Author

Not really sure what to do about this.

Can you slap on a class which changes the hovering color? Since the bg is always blue, it should be independent of the theme.

Done

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thank you!

@hotzenklotz hotzenklotz merged commit 830ff9e into master Feb 15, 2024
2 checks passed
@hotzenklotz hotzenklotz deleted the fix-welcome-notification branch February 15, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants