-
Notifications
You must be signed in to change notification settings - Fork 259
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
Add promo cards to HomeScene #4687
Conversation
02b0d87
to
35b3150
Compare
<EdgeAnim style={{ height: theme.rem(11.5) }} enter={{ type: 'fadeInUp', distance: 30 }}> | ||
<CarouselUi4 height={theme.rem(10)} width={screenWidth}> | ||
{promos.map(promoInfo => ( | ||
<PromoCardUi4 navigation={navigation} promoInfo={promoInfo} key={promoInfo.localeMessages[0]} /> |
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.
Shouldn't this be localeMessages[language]
or something like that
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.
It's just the key and we're guaranteed the first localeMessage
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.
But we shouldn't be using the first message. We should be using the one that matches the language.
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.
We should be using the language, not the first entry. That's why we even have a localeMessage in the first place.
35b3150
to
f54125a
Compare
Close functionality is broken in that the Punted and created a spinoff task: https://app.asana.com/0/1200382638405084/1206315466361645/f |
1d206b3
to
201713e
Compare
src/components/ui4/PromoCardsUi4.tsx
Outdated
if (osVersions != null && osVersions.length > 0 && !osVersions.includes(osVersion.toString())) return false | ||
|
||
// Validate date range | ||
const startDate = startIsoDate != null && !isNaN(Date.parse(startIsoDate)) ? new Date(startIsoDate) : null |
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.
Iso dates can be lexographically compared. No need to convert with new Date
. Just do
const currentIsoDate = new Date().toISOString()
if (startIsoDate > currentIsoDate) return false
if (endIsoDate < currentIsoDate) return false
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.
Fix android version, use v1/rollup, use proper language
8eca0c3
to
a16e808
Compare
a16e808
to
26d9544
Compare
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.
I added a commit to clean the osVersion into semver. After rebasing on develop, visuals look broken. Needs to be fixed before merge.
c1bf673
to
bca3f70
Compare
d85b635
to
60e971a
Compare
Clean up style logic and add mini button. - Rename styles to make it more clear. - Since we need different padding and height for mini buttons, make sure marginRem and paddingRem are always last in the styles list so they can continue to override built-in layouts.
Per UI4's split 0.5rem spacing expectations, roll those margins into CarouselUi4 so it always goes outside of those margins horizontally to fill up 100% of the width.
- Fixed height, accomodating a maximum of 4 lines of text. Potentially adjusted once average content length is given. - No text shrinking for readability.
Pending close button functionality
60e971a
to
1425f6f
Compare
Pending close button functionality
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: