-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Feature: Create FeaturedTopic and FeaturedTopicsRow Components #2237
base: topic-landing
Are you sure you want to change the base?
Feature: Create FeaturedTopic and FeaturedTopicsRow Components #2237
Conversation
…es of hitting 3 with descriptions
static/css/s2.css
Outdated
margin-inline-start: 30px; | ||
margin-top: 19px; | ||
width: 207.593px; | ||
/*height: 140.617px;*/ |
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.
remove if we dont need it
margin-inline-end: 30px; | ||
margin-inline-start: 30px; | ||
margin-top: 19px; | ||
width: 207.593px; |
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.
why a decimal point?
.topic-card-with-description .cardDescription{ | ||
margin-inline-end: 30px; | ||
margin-inline-start: 30px; | ||
margin-top: 19px; |
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.
these numbers for width and height seem pretty random, also on top seems like there are very specific numbers. ideally the cards wont have a strict width and height but rather a min-width min-height. Sefaria in general tries to stick to multiples of 5 for margins and width/height
static/css/s2.css
Outdated
} | ||
.topic-card-with-description .card{ | ||
flex: 1; | ||
border-top: 4px solid var(--Sefaria-Blue, #18345D); |
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.
why are we adding the color as a default, I believe --Sefaria-Blue should always be defined
|
||
const fetchRandomTopicDeck = async () => { | ||
const poolName = Sefaria.getLangSpecificTopicPoolName('general'); | ||
const topics = await Sefaria.getTopicsByPool(poolName, Math.pow(numTopics, 3)); |
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.
can you explain why x^3?
Description
New Component:
RandomTopicCardWithDescriptionRow
Fetches a set of random topics with descriptions.
Displays topics in a responsive card row format, including links to learn more.
Code Changes
The following changes were made to the files below
Notes
Any additional notes go here