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

Clarify dApp landing page #287

Merged
merged 5 commits into from
Feb 6, 2024

Conversation

timothymcmackin
Copy link
Collaborator

@timothymcmackin timothymcmackin commented Jan 29, 2024

Nic had some comments after #221 was merged. This PR includes them.

@timothymcmackin timothymcmackin self-assigned this Jan 29, 2024
@timothymcmackin timothymcmackin requested a review from a team as a code owner January 29, 2024 15:35
Copy link

vercel bot commented Jan 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs-staging ✅ Ready (Inspect) Visit Preview Jan 30, 2024 1:51pm

@NicNomadic
Copy link
Collaborator

Sorry, I don't see any difference in the preview at https://docs-staging-git-nic-comments-on-dapp-landing-page-trili-tech.vercel.app/dApps. Am I looking at the right place?

@NicNomadic NicNomadic self-requested a review January 30, 2024 08:31
docs/dApps.mdx Outdated

<LucidDiagram width="640px" height="480px" src="https://lucid.app/documents/embedded/8caf9ef1-11e4-454a-bbb6-ef4852515959" id=".4NXymECcQqz" />
<LucidDiagram width="640px" height="360px" src="https://lucid.app/documents/embedded/8caf9ef1-11e4-454a-bbb6-ef4852515959" id=".4NXymECcQqz" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Referring to an artifact which is editable online separately from any github control seems a real flaw, isn't it? Why don't you just export your picture as a static image, and perhaps link in a comment to the source, in case one wants to edit it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I put it in as a static image. In cases of large diagrams I like offering the LucidChart container because people can scroll and zoom to explore the diagram, but yes, it means that the artifact is not in the repository.

Copy link
Collaborator

@NicNomadic NicNomadic Jan 30, 2024

Choose a reason for hiding this comment

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

It's much better like this, both from usability and security viewpoints.

People can zoom in without the Lucid source, isn't it? If you mean that they would have more resolution, maybe you can try to improve that in the static image, either by increasing its resolution, or by exporting SVG (which is supposed to be more scalable, as the name implies).

@timothymcmackin timothymcmackin merged commit b7ffb51 into staging Feb 6, 2024
4 checks passed
@timothymcmackin timothymcmackin deleted the nic-comments-on-dapp-landing-page branch February 6, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants