Move font and text style to theme#90
Conversation
SjaakSchilperoort
force-pushed
the
feature/add-font-and-text-to-theme
branch
from
8e9aa1c to
8ed8511
Compare
|
|
||
| const onToggle = (index: number, isExpanded: boolean) => { | ||
| Log.debug('Accordion:onToggle', index, isExpanded) | ||
| unsafeLayoutAnimation('Accordion:onToggle') |
There was a problem hiding this comment.
Dit is het enige wat me opvalt. Is de layout animation niet (meer) nodig of vangen we dit op een andere manier op? Ik heb bijvoorbeeld naar de animaties van ChallengeContentItems op de challange-resultaten in ObsIdentify gekeken en ik kon geen verschil vinden.
There was a problem hiding this comment.
De animaties zijn wel nodig, maar bij overgang naar de new architecture van React Animation hebben we gebruik van LayoutAnimation op veel plekken verwijderd. Het leidde tot app crashes wegens gebruik van een component die niet meer was gemount. De app gedraagt zich op die plekken niet zo vloeiend meer. De oplossing is om deze animates te herimplementeren met reanimated.
Ik dacht dat het gebruik van layout animation in de components library een left over was die we over het hoofd hadden gezien, maar ik zie dat Observation het op nog meer plekken gebruikt. Ik zal het verwijderen van layout animation in de components library terugdraaien, en dan kunnen we separaat beslissen hoe we hiermee om gaan.
2 participants
Resolves: #89
textStyleto theme astheme.textfontto theme.src/stylesstill has the static type for font. When the apps have switched completely to dynamic theme, the static type forfontcan be removed@observation.org/react-native-componentsfor UI components@observation.org/react-native-components/themefor theme-related classes, types and objects@observation.org/react-native-components/stylesfor static stylespackage.json