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.

Sign up for GitHub

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

Jump to bottom

[ui] promote uri, relation id fields in catalog #22293

Merged
merged 3 commits into from
Jun 5, 2024
Merged

[ui] promote uri, relation id fields in catalog #22293

merged 3 commits into from
Jun 5, 2024

Conversation

benpankow
Copy link
Member

@benpankow benpankow commented Jun 5, 2024 •
edited
Loading

Summary

Promotes the URI and Relation identifier metadata fields to the Definitions header in the asset page.

Screenshot 2024-06-04 at 8 24 41 PM

Test Plan

Tested locally.

@benpankow
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @benpankow and the rest of your teammates on Graphite Graphite

@benpankow benpankow requested review from hellendag, sryza and salazarm June 5, 2024 03:32
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 5, 2024 •
edited
Loading

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-mdaj5e9a3-elementl.vercel.app
https://benpankow-highlight-uri-relation-id-fields.core-storybook.dagster-docs.io

Built with commit e4452d3.
This pull request is being automatically deployed with vercel-action

@sryza
Copy link
Contributor

sryza commented Jun 5, 2024

@braunjj what do you think?

@sryza
Copy link
Contributor

sryza commented Jun 5, 2024

Can we make the relation identifier easily copy-able?

@benpankow
Copy link
Member Author

Screenshot 2024-06-05 at 9 02 52 AM

good call

salazarm
salazarm approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@salazarm salazarm left a comment

Choose a reason for hiding this comment

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

nice

@sryza
Copy link
Contributor

sryza commented Jun 5, 2024

Elsewhere, we use this little copy icon:
image

Would that make sense here?

@braunjj
Copy link
Contributor

braunjj commented Jun 5, 2024

I think we should do something like this

  • Use default text color (not a link)
  • Use the icon called content-copy (it drives me crazy that we use a "paste" icon for copy in the asset key
  • Tooltip says "Copy" when you hover over the button
    image

@braunjj
Copy link
Contributor

braunjj commented Jun 5, 2024

@benpankow @sryza could we actually do something like this where we combine URI/relation identifier and the storage kind icon into one section called "Storage"
image

@sryza
Copy link
Contributor

sryza commented Jun 5, 2024 •
edited
Loading

@braunjj I like that

@benpankow benpankow merged commit 6f9433e into master Jun 5, 2024
2 checks passed
@benpankow benpankow deleted the benpankow/highlight-uri-relation-id-fields branch June 5, 2024 22:19
salazarm pushed a commit that referenced this pull request Jun 10, 2024
@benpankow @salazarm
[ui] promote uri, relation id fields in catalog (#22293)
ea5ca15 
## Summary

Promotes the URI and Relation identifier metadata fields to the
Definitions header in the asset page.

<img width="335" alt="Screenshot 2024-06-04 at 8 24 41 PM"
src="https://github.com/dagster-io/dagster/assets/10215173/3f95e9a1-9738-4d09-9609-633d9c970c24">


## Test Plan

Tested locally.
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
@benpankow @danielgafni
[ui] promote uri, relation id fields in catalog (dagster-io#22293)
a1e1cb6 
## Summary

Promotes the URI and Relation identifier metadata fields to the
Definitions header in the asset page.

<img width="335" alt="Screenshot 2024-06-04 at 8 24 41 PM"
src="https://github.com/dagster-io/dagster/assets/10215173/3f95e9a1-9738-4d09-9609-633d9c970c24">


## Test Plan

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

@salazarm salazarm salazarm approved these changes

@hellendag hellendag Awaiting requested review from hellendag

@sryza sryza Awaiting requested review from sryza

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
@benpankow @sryza @braunjj @salazarm