-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
add entity peek ahead component #15014
Conversation
Signed-off-by: Brian Fletcher <brian@roadie.io>
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
Changed Packages
|
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
We have the option of using the color of the text, chips or italics to allow the reader to understand the information a little better. |
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.
Cool stuff!
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Brian Fletcher <brian@roadie.io>
…-entity-peek-ahead
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
That MUST be a builtin feature of |
@freben I havent quite found that setting yet. But let me drill further. |
There is an open issue for this jcoreio/material-ui-popup-state#85 |
Signed-off-by: irma12 <irma@roadie.io>
@freben I have added a prop 'delayTime' so we should be able to make it work now 🤞🏼 |
Uffizzi Preview |
Signed-off-by: irma12 <irma@roadie.io>
Thanks Irma for helping me out. I felt a little stuck there. @freben I am noticing that gmail has a delay of 1.5 seconds and github has a delay of less than a second. Irma has set the default value to 0.5 seconds. Let us know if you would like it longer. |
Signed-off-by: Brian Fletcher <brian@roadie.io>
…-entity-peek-ahead-irma
Signed-off-by: Brian Fletcher <brian@roadie.io>
…eHQ-entity-peek-ahead
Signed-off-by: Brian Fletcher <brian@roadie.io>
Changed Packages
|
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.
Still some behavioural issues I think
<Card> | ||
<CardContent> | ||
<Alert severity="warning"> | ||
{entityRef} was not found {error?.message} |
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.
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.
Done.
|
||
return ( | ||
<> | ||
{' '} |
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.
probably unintentional?
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.
I think so. Its removed now.
}} | ||
> | ||
<> | ||
{loading && <Progress />} |
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.
note in the story how the progress looks (stacked at the top with no padding), it should probably be inside a card content as well
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.
done.
{' '} | ||
<span | ||
onMouseEnter={debouncedHandleMouseEnter} | ||
onMouseLeave={handleOnMouseLeave} |
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.
This doesn't seem to work, did you try it out carefully in the storybook? You can't enter the popover card at all; it closes if you try. If you try to slowly go with the mouse pointer into the test button in the storybook from below, the popover never shows.
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.
I tried to fix this. Could you take a look now.
export const EmailCardAction = ({ email }: { email: string }) => { | ||
return ( | ||
<Tooltip title={`Email ${email}`}> | ||
<Button target="_blank" href={`mailto:${email}`} size="small"> |
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.
To get it behaviorally the same as our links, could you try out whether <IconButton component={Link} to={<url>} size="small">
works as intended? I think it might. See for example
component={Link} |
Using our Link of course, not the one in MUI. Adding aria-label is bonus points
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.
done
return ( | ||
<> | ||
<Tooltip title="Show details"> | ||
<Link |
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.
should this also have been IconButton
and component={Link}
?
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.
done.
import { CompoundEntityRef } from '@backstage/catalog-model'; | ||
import { entityRouteRef } from '../../routes'; | ||
import { CatalogApi } from '@backstage/catalog-client'; | ||
|
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.
I think it would be really valuable to have a table as one of the stories, to get a feeling for how it feels in a context where there are lots of links and getting some of the overlap problems visible if any
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.
done.
…-tttentity-peek-ahead
Signed-off-by: Brian Fletcher <brian@roadie.io>
Signed-off-by: Brian Fletcher <brian@roadie.io>
(just popping in to mention I'm off for the holidays and reviews might take a while longer) |
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.
Alright, let's
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Signed-off-by: Brian Fletcher brian@roadie.io
Hey, I just made a Pull Request!
add entity peek ahead component
closes #13678
It looks like this:
or for a group / user with an email it looks like this:
✔️ Checklist
Signed-off-by
line in the message. (more info)