3418: Add badges for news source - #4262
Conversation
4f9b79d to
a503074
Compare
a503074 to
52477e5
Compare
90aae48 to
b5a521a
Compare
bahaaTuffaha
left a comment
bahaaTuffaha
left a comment
There was a problem hiding this comment.
At testumgebung (native and web) when it's set to all it keeps repeating (by infinite scroll) the same items due to duplicate keys:
react-dom-client.development.js:6604 Encountered two children with the same key, `local-19600`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
- ❓ Are they depend on id for the key? not sure if this just because of the dummy data.
- Switched to cms but there is no local news for Augsburg (is that expected). also in native when I switch to cms then select augsburg it says unknown error occurred (this could be unrelated to this PR).
- 🔧 At web: the filter can be set to null if I selected the same toggleButton twice.
- The tuNews logo got changed:
before:
After:
- for RTL languages I think we need to flip the Chip with the title:
|
Thank you for your PR 😻 I noticed, in iOS, filter has a weird shadow between local and national news, see the screenshot: |
b5a521a to
07ffdd9
Compare
07ffdd9 to
4d09547
Compare
I removed pagination since we are limiting all news to 28 days.
If nobody ever created local news there (in the last 28 days), yes, its intended. /testumgebung/de works as expected for me.
Fixed.
Yes, it was intended. However, I now moved the logo to the end of the page.
Fixed, good catch. |
I can neither reproduce this in the native nor in the web app on an iOS simulator 🤔 |
4d09547 to
d7b310d
Compare
bahaaTuffaha
left a comment
bahaaTuffaha
left a comment
There was a problem hiding this comment.
Great work! ✅
There some questions I have left 👇
- Would it be better to take full width for mobile screens on web to match native ?
| size='small' | ||
| value={value} | ||
| exclusive | ||
| onChange={(_, value) => setValue(value ?? value)} |
There was a problem hiding this comment.
🔧 Are these both coming from the parameter ? I think it needs renaming here
| titleNumberOfLines={0} | ||
| descriptionNumberOfLines={0} | ||
| title={ | ||
| <View style={Styles.titleRow}> |
There was a problem hiding this comment.
| <View style={Styles.titleRow}> | |
| <View style={[Styles.titleRow, { flexDirection: contentDirection(languageCode) }]}> |
| <View style={Styles.bottomInfo}> | ||
| <Text variant='body2'> | ||
| <TimeStamp lastUpdate={newsItem.lastUpdate} showText={false} /> | ||
| </Text> | ||
| <View style={Styles.bottomInfo}> | ||
| <Text variant='body2' style={{ paddingVertical: 8 }}> | ||
| <TimeStamp lastUpdate={newsItem.lastUpdate} showText={false} /> | ||
| </Text> | ||
| <ReadMoreWrapper language={i18n.language}> | ||
| <Text | ||
| variant='h6' | ||
| onPress={navigateToNews} | ||
| style={{ | ||
| marginTop: 4, | ||
| color: theme.colors.primary, | ||
| }}> | ||
| {t('common:more')} | ||
| </Text> | ||
| </ReadMoreWrapper> | ||
| </View> | ||
| </Description> | ||
| } | ||
| onPress={navigateToNews} | ||
| accessibilityLanguage={languageCode} | ||
| role='link' | ||
| style={{ flexDirection: 'column' }} | ||
| /> | ||
| </ListItemWrapper> | ||
| </View> |
There was a problem hiding this comment.
❓ Do we still need this wrapper (bottomInfo) ?
| const styles = StyleSheet.create({ | ||
| chip: { | ||
| borderRadius: 32, | ||
| borderColor, | ||
| backgroundColor: theme.colors.background, | ||
| alignSelf: 'flex-start', | ||
| }, | ||
| }) |
There was a problem hiding this comment.
It's better to be outside of the component body and use style array for the dynamic ones.
There was a problem hiding this comment.
Is it ok to have another release note? 🤔
you are right, now it is gone. Probably I checked the parent branch... |
3 participants
Short Description
Add badges for news source.
Proposed Changes
morehintSide Effects
N/A
Checklist
Testing
Test the whole news feature on web and native. Test native also offline. Test pagination. Check the labels, apply the filters and navigate to news details.
Resolved Issues
Fixes: #3418