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

[Re-architecture]: Rendering Of Cards #1633

Open
anuraghazra opened this issue Mar 6, 2022 · 6 comments
Open

[Re-architecture]: Rendering Of Cards #1633

anuraghazra opened this issue Mar 6, 2022 · 6 comments
Labels
core-team Feature that requires the attention of the GRS core team. enhancement New feature or request. proposal Proposals to change code or documentation fundamentals. ⭐ top feature Top feature request. ⭐ top issue Top issue.

Comments

@anuraghazra
Copy link
Owner

anuraghazra commented Mar 6, 2022

The Problem

The current architecture of rendering the card follows a more traditional approach where we concatenate raw svg strings to build a valid SVG markup.

  • Build svg template
  • Manipulate string
  • Interpolate dynamic props
  • Render card

Problem 1

The problem with this approach is that it's not very scalable specially when we consider the fact we have to handle precise pixel values and by default SVGs don't support flexible layouts, text wrapping and other vital layout engine features which are needed to build a complex card layout.

This approach has implemented since the start of github-readme-stats which worked fine early on, but as the cards grow in complexity it's becoming more and more harder to manage the layouts. For example we had implement all sorts of hacky workarounds to use FlexBox, TextWrapper, MeasureText and other features which are already present in any modern layout engine.

Problem 2

Second point is that manipulating strings like we do now poses a huge burden for us to properly interpolate all the pixel values which are needed and it's a manual process. There is no %, rem, em, or flexible units to work with in SVGs. Every single element on the SVG has to be carefully crafted to maintain their size/width/height/position. It's not fun to build cards.

Solution

The solution which I'm proposing to implement is by taking advantage of foreignObject in SVG.

Advantages of foreginObject

  • Easy to build cards, we have all the power of flexible layouts, CSS & browser's layout engine.
  • Browser support is good
  • Trivial to build templates for cards

Low hanging questions

There are few things which we have to check first, one of them is if foreignObject is supported in github mobile or not, That's an issue because we don't know what kind of engine the github mobile app using.

Other thing is checking various browsers for compatibility and consistency of the styling and layout. Even though browser support is good, we still need to do some testing to make sure it's consistent across browsers.

Implementation plan

Over the next few weeks I'll be start implementing this feature and deploy it in a vercel preview link to test things out.

Feedback

I'll be needing feedback from the community and the core members (@rickstaa) to implement this & the feasibility of this.
Let me know what you folks are thinking about this approach we can discuss more on the pros and cons.

@anuraghazra anuraghazra pinned this issue Mar 6, 2022
@rickstaa rickstaa added the proposal Proposals to change code or documentation fundamentals. label Mar 9, 2022
@rickstaa
Copy link
Collaborator

@anuraghazra I like your proposal wasn't aware of foreignObject object, but it looks like the right tool for the job! I think it might clean up the codebase! And if not, it will save us hours trying to interpolate pixel values 🥲.

Feel free to tag me if you need my help.

@rickstaa rickstaa added enhancement New feature or request. and removed feature labels Jul 25, 2022
@rickstaa rickstaa unpinned this issue Aug 16, 2022
@b0o
Copy link

b0o commented Oct 13, 2022

Perhaps satori might be another solution?

@rickstaa
Copy link
Collaborator

@b0o Thanks for your suggestion https://github.com/vercel/satori is indeed a good solution. @anuraghazra started experimenting with it already 🚀.

This is amazing.

It'll potentially be a game changer for github-readme-stats. Can't wait to make a prototype with it.https://t.co/RppwBrKlBC

— Anurag Hazra ⚛ (@anuraghazru) October 11, 2022

@g-nitin-1
Copy link

if this issue still remains please let me know

@rickstaa
Copy link
Collaborator

if this issue still remains please let me know

It seems like @anuraghazra hasn't implemented this change yet. Given its complexity and the fact that it spans the entire codebase, I believe it might be best handled by someone from the core GRS team 😅. I've taken off the hacktoberfest label for now. @anuraghazra, if you believe the community can tackle this feature, feel free to reapply the label and assign it to @g-nitin-1. 👍🏻

@anuraghazra
Copy link
Owner Author

anuraghazra commented Oct 20, 2023

There are a lot of gaps that still need to be filled in and explore certain things. It's best we discuss this and plan this feature diligently from GRS core team.

@rickstaa rickstaa added the core-team Feature that requires the attention of the GRS core team. label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-team Feature that requires the attention of the GRS core team. enhancement New feature or request. proposal Proposals to change code or documentation fundamentals. ⭐ top feature Top feature request. ⭐ top issue Top issue.
Projects
None yet
Development

No branches or pull requests

4 participants