-
Notifications
You must be signed in to change notification settings - Fork 7
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
Copy Tooltip from nns-dapp #367
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add an e2e test, which are easy to add here.
a6e487f
to
7b52268
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also have a screenshot once we fix the issue, right?
What do you mean? Did you not see the screenshot? |
Sorry, I meant a screenshot testing of the issue once it's fixed. |
Actually, the e2e test I added observes the issue. But once it's fixed, the issue of course won't be visible in the screenshot because then the issue doesn't exist anymore. |
I would expect a working screenshot that in case we break it again we find it early. |
Oh, I see. I don't think that's what screenshots are for. And also it's more likely that some cases we didn't think of are still/already broken rather than that some obvious case break. And it would be too many screenshots to cover even a decent fraction of cases. The right way forward is to factor the logic outside of the component and have lots of unit tests with numbers. |
If we can make it as unit tests better, yes. |
Motivation
NNS dapp has a tooltip component with somewhat broken behavior.
Before fixing it, I thought it would be useful to have a showcase in gix-components to easily play around with it while changing it.
And it seemed like the kind of component that would be useful to have in gix-components anyway.
So before fixing the tooltip component, I'm copying it unchanged from nns-dapp to gix-components.
Once this component is fixed, we can switch nns-dapp over to this tooltip component.
Changes
Tooltip.svelte
,Tooltip.spec.ts
andTooltipTest.svelte
from thenns-dapp
repo unchanged.Screenshots