Add basic setup for showing open source projects #693
Conversation
Siilwyn
force-pushed
the
basic-os-projects
branch
2 times, most recently
from
831cffc to
0d33f4c
Compare
Siilwyn
force-pushed
the
basic-os-projects
branch
from
0d33f4c to
bc0936b
Compare
There was a problem hiding this comment.
Since @lsarni also had some thoughts if we should make this pretty specific component, I think we should have a look with the three of us how to approach this. I'll get back to you guys tomorrow.
| await client.fields.create(sectionOpenSourceProjectItemModel, { | ||
| label: 'Description', | ||
| api_key: 'description', | ||
| field_type: 'string', |
There was a problem hiding this comment.
Could we make this structured text? Then it's more flexible in the future for deciding which content goes in it. Maybe we can also allow h3, so the title could optionally be added, without needing a designated field for it. Which in turn makes the component more flexible.
There was a problem hiding this comment.
Think I want to enforce a title, but having a structured text here could be nice yes 👍
| <slot /> | ||
| <LinkWithTrailingSlash | ||
| :class="{ | ||
| body: Boolean(variant), |
There was a problem hiding this comment.
So the font size and line height are dependent of the app link being underlined?
Since the class is being set on the root element of the LinkWithTrailingSlash, which also is the root element of the AppLink, could we maybe add the body class on the open source projects component? Then the typography styles are more flexible.
There was a problem hiding this comment.
So the font size and line height are dependent of the app link being underlined?
Yes
could we maybe add the body class on the open source projects component
I'm unsure, I understand the extra flexibility but don't you think that having body styling by default covers all cases where a link needs to look like a link? Actually what do you think of only adding the body class when the underline variant is given?
There was a problem hiding this comment.
I would prefer only adding the body class for the underline variant. I prefer making develop intentions clear. And we're not sure if the body class is needed for another future variant. Adding it already might make other developers wonder if it breaks anything to remove the body class.
| > | ||
| <span | ||
| v-if="variant === 'underline'" | ||
| class="app-link__icon" |
There was a problem hiding this comment.
Can we name this call app-link__content or so? IMO it's not really the icon but actually the content, after which we append the arrow icon.
What changes were made
How to test or check results
/en/open-source
Checks