Skip to content

Conversation

remidinishanth
Copy link
Contributor

@remidinishanth remidinishanth commented Jun 14, 2025

Currently, the color links are broken in design.md file

The old link is giving the error Cannot proxy the given URL

Fixing the same, also keeping the hex code as reference.

Before

image

After

image

@daichimukai
Copy link
Contributor

Hi @remidinishanth,

Thank you for submitting your PR. I appreciate your efforts.

I have a suggestion regarding the handling of user-uploaded files. It can be challenging to manage these files since only the uploader knows their origin and purpose. To ensure consistency and ease of management, I recommend storing the necessary image files directly in the repository. This approach will make it easier for everyone to access and maintain them.

Additionally, I believe the alt attribute is sufficient for accessibility purposes. Here's a proposed change to your patch:

- - ![#FFFF00](https://via.placeholder.com/15/FFFF00/000000?text=+) TopoLVM components
- - ![#ADD8E6](https://via.placeholder.com/15/ADD8E6/000000?text=+) Kubernetes components
- - ![#FFC0CB](https://via.placeholder.com/15/FFC0CB/000000?text=+) Kubernetes CSI Sidecar containers
+ - ![#FFFF00](./img/FFFF00.png) TopoLVM components
+ - ![#ADD8E6](./img/ADD8E6.png) Kubernetes components
+ - ![#FFC0CB](./img/FFC0CB.png) Kubernetes CSI Sidecar containers

Signed-off-by: Nishanth Reddy <remidinishanth@gmail.com>
@remidinishanth
Copy link
Contributor Author

Thanks for the comment @daichimukai , I've updated, Please take another look.

Copy link
Contributor

@daichimukai daichimukai left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@pluser pluser left a comment

Choose a reason for hiding this comment

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

Nice work!

@pluser pluser merged commit 7a728d1 into topolvm:main Jun 19, 2025
30 of 34 checks passed
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.

3 participants