[#394] feat: update link of pycontw blogger #395
Conversation
✅ Deploy Preview for ephemeral-sable-89c7e0 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
yungshenglu
requested review from
SivanYeh and
josix
and removed request for
josix
|
@tim08094495757 @SivanYeh @erik1110 please take a look about the refactroing (extracting data layer, new css coding style). Feel free to leave any thoughts, thanks! |
There was a problem hiding this comment.
Thanks, @yungshenglu and @josix.
These are feedbacks:
- In BulletinCard.vue, classes like bulletin--card__content were removed, but classes like bulletin--card__header are still survived~ => Need a explanation.
- Youtube.svg to YouTube.svg => Now YouTube.svg is missing.
|
There was a problem hiding this comment.
Thanks for the hard work @yungshenglu. The whole refactoring about static data control looks really neat. Thanks!
configs/pageLanding.js
Outdated
| @media (min-width: 768px) { | ||
| .core-footerIcon { | ||
| width: 33px; | ||
| height: 33px; | ||
| } | ||
| } |
There was a problem hiding this comment.
question: I found that we have defined the md breakpoint prefix as @media (min-width: 768px) {....} in tailwind.config.js. I guess we could just adopt utility class md:w-[33px] md:h-[33px]for the class core-footerIcon?
There was a problem hiding this comment.
If we have already defined, then I can replace them.
There was a problem hiding this comment.
| @apply absolute; | ||
| top: 30%; | ||
| } | ||
|
|
||
| .bulletin--card__header { | ||
| @apply font-serif text-base text-center mt-6 mb-5 font-bold; | ||
| @media (min-width: 375px) { |
There was a problem hiding this comment.
question: how about using xs:text-xl xs:mt-8 xs:mb-5
| <div class="w-full flex justify-center items-center flex-wrap"> | ||
| <ext-link | ||
| v-for="(item, index) in dataConfig" | ||
| :key="`landing-footer-icon-${index}`" |
There was a problem hiding this comment.
suggestion: I guess we also need to define the convention of naming the keys of items in array. e.g. [page]-[block]-[element]-[index] or something like that.
There was a problem hiding this comment.
This is the part I forgot in the last meeting. There are so many kinds of naming convention in the repository. I totally agree that we should define one of our naming convention here.
Types of changes
Description
BulletinCardCollection和BulletinCardconfigs/pageLanding.jsFooterSocial和FooterHistoryconfigs/pageLanding.jsRelated Issue
#394
Additional context
NA