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

Update kit logos #1113

Merged
merged 44 commits into from
Dec 15, 2024
Merged

Conversation

Grand-Thibault
Copy link
Contributor

@Grand-Thibault Grand-Thibault commented Dec 12, 2024

Description

  • all KIT logos and gallery banners are available as svg
  • all references are updated

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

jSchuetz88

This comment was marked as resolved.

Copy link
Contributor

@matbmoser matbmoser left a comment

Choose a reason for hiding this comment

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

Make sure there is no svg water mark. This usually happens when it was moved to PowerPoint

@Grand-Thibault
Copy link
Contributor Author

Looks great, but you missed a setting for the connector image: image

I fixed it, thx

@Grand-Thibault
Copy link
Contributor Author

Make sure there is no svg water mark. This usually happens when it was moved to PowerPoint

I moved it to Powerpoint --> no watermark --> all fine

@Grand-Thibault
Copy link
Contributor Author

@matbmoser @jSchuetz88 @stephanbcbauer I just added kit-colors.md. Please review it as well.

@ClosedSourcerer
Copy link
Contributor

@Grand-Thibault

Could you provide me with the source for one or two of the images please?
I would like to try to generate the SVGs myself.
Some of the files came from draw.io appearently, while for others I was not able to determine the source.

I randomly checked 10 of the newly added .svg files and noticed the following:

  • The file size seems way too big for truely optimized vector graphics or is at least inconsistent.

This lead me to have an indepth look into static\img\kits\agents\agents-kit-gallery.drawio.svg

  • SVG with embedded Base64 encoded image data.
  • Decoding the base64 leads me to believe, that generated SVGs are way too overcomplicated and contain too much data.

Funnily enough:

  • demand-and-capacity-management-kit-logo.drawio.svg is only 10KB in size.
  • data-chain-kit-logo.drawio.svg is 216 KB in size. for pretty much the same picture:

image

BUT demand-and-capacity-management-kit-logo.drawio.svg is an svg with embedded PNG data. So at least partially it is not true vector graphic.

  • Embedding a whole font into most of the SVGs (Especially the ones that do not contain any text) seems to also have contributed to the filesize.

@ClosedSourcerer
Copy link
Contributor

ClosedSourcerer commented Dec 13, 2024

Regarding the newly added: demand-and-capacity-management-kit-logo.drawio.svg in 92d1526

That file ist at least two-thirds data-garbage.

I made a quick manual edit to it.

The file started out at 30KB:
demand-and-capacity-management-kit-logo drawio

I added roughly 3kb data by adding human readable formatting.
And then removed 19 KB by throwing out alot (not even all) of the garbage:
CLEANED

The garbage mainly consist of data-cell-id attributes, which according to SVG2 Spec may exist as custom attributes but they serve no purpose in the file. At least, no purpose that benefits the tractus-x homepage.

Copy link
Member

@jSchuetz88 jSchuetz88 left a comment

Choose a reason for hiding this comment

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

Looks good from my side now. Build and tested it.

@jSchuetz88
Copy link
Member

jSchuetz88 commented Dec 13, 2024

Regarding the newly added: demand-and-capacity-management-kit-logo.drawio.svg in 92d1526

That file ist at least two-thirds data-garbage.

I made a quick manual edit to it.

The file started out at 30KB: demand-and-capacity-management-kit-logo drawio

I added roughly 3kb data by adding human readable formatting. And then removed 19 KB by throwing out alot (not even all) of the garbage: CLEANED

The garbage mainly consist of data-cell-id attributes, which according to SVG2 Spec may exist as custom attributes but they serve no purpose in the file. At least, no purpose that benefits the tractus-x homepage.

From my perspective, regardless of whether you are right (on the technical level), I don't see any reason why we should discuss the generation of .svg files by .drawio (or any other established software) here. If we expect this from our contributors to check and fix on the data structure level, we most likely will never see a contribution again. Content wise, this PR is an improvement compared to the previous state, it cleans up the file-structure + it works fine and looks good.

In addition, I wouldn't say it does not serve any purpose. I may be wrong here, but in my understanding, it is the "garbage" portion that allows the file to be edited in draw.io again by everyone else if required.

@matbmoser matbmoser closed this Dec 14, 2024
@matbmoser matbmoser reopened this Dec 14, 2024
Copy link
Contributor

@matbmoser matbmoser left a comment

Choose a reason for hiding this comment

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

No more discussions. Please merge ASAP.

Thank you @Grand-Thibault for your wonderful work!

@matbmoser
Copy link
Contributor

dismissing review from @ClosedSourcerer since the changes were included by @Grand-Thibault! Thx guys.

@matbmoser matbmoser dismissed ClosedSourcerer’s stale review December 15, 2024 13:02

Because the changes are old, and they were implemented already.

@matbmoser
Copy link
Contributor

matbmoser commented Dec 15, 2024

I am merging this ASAP, if anything is missing please open another PR @ClosedSourcerer. The other kits logos are fine. Great job @Grand-Thibault 🚀

@matbmoser matbmoser merged commit c4cce7e into eclipse-tractusx:main Dec 15, 2024
9 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.

4 participants