-
Notifications
You must be signed in to change notification settings - Fork 111
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
Link groups front-end refactor #65
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic, just ran it locally and it works great! 🤩
@klinegareth once this is merged we can start adding more metadata on the coding challenges and tracks we've started. @fdoflorenzano let's remember to revisit CT characters in lieu of emojis later. Also, is there anything from #66 to be incorporated here? For example, would having mini thumbnails of the videos work well? Or is that adding too much?
Shall I go ahead and merge? Yay!
@shiffman We could so similar stuff as in #66 here too, tho images are a bit harder to handle as I've mentioned before and the most direct way of doing it would be doing more folders inside and maybe doing a file per link, which seems like a lot for them. Also I would be worried that code examples and links start looking too much a like visually. We can leave that question pending with the emojis too. You can merge, but I would prefer to wait and know that @klinegareth read my note at the end of de description. |
Great! I added a review request for @klinegareth! Great points about the images, I think the examples are the priority so we can leave this idea to the side for now. I wonder what the YouTube thumbnails would look super tiny if we pulled those in via the API? |
Amazing! I left the mentioned ideas for images/icons/thumbnails in issue #69 for us to revisit at some point. |
Gatsby Cloud Build Reportthecodingtrain.com 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 26s PerformanceLighthouse report
|
@klinegareth and I are meeting and went through this, and we're ready to merge! I'm going to merge it now then Kline will take |
As proposed in #63, this revisits how links in reference tabs look and how they're composed. In favor of a simple
"icon"
and"description"
properties instead of"author"
, this looks to fill the space better and allow a more flexible way of describing references.For now,
"icon"
is just an optional an simple string. The front-end is built as it would expect a simple emoji, not a long string. We can leave pending adding Coding Train characters here, but we gotta know what's the status with them."description"
also is an optional string, can be relatively long, but they work better short and snappy. They can contain HTML tags that will be parsed, so that links can be added inside. For example, the following descriptions mentions two authors and links to their respective websites:Here are some screenshots of how the front-end looks like in different screen sizes:
This also updates tests to fit this new property schema, and updated manually all files that had
"author"
.@klinegareth I also went ahead and moved the
index.json
fromcontent/videos/tracks
from there cause not only it cause an the unexpected error, it also cause that tests inside of it didn't continue going forward, and that made the script ignore a lot of content. I figured you didn't remove it because it could be used as a template for new videos, so I moved it to a new location:content/templates/videos
. The templates folder can be used to leave some template JSONs for different content. Tracks and contributions can also have one. I left the testing rules for that folder more loose in case you want to change it. Let me know if this is fine!