New component library #1
Conversation
jonathan-waarneming-nl
force-pushed
the
feature/new-component-library
branch
from
43874eb
to
ffbaae4
Compare
jonathan-waarneming-nl
force-pushed
the
feature/new-component-library
branch
from
d9aff18
to
27f0175
Compare
jonathan-waarneming-nl
force-pushed
the
feature/new-component-library
branch
from
afe4a0c
to
dca4df9
Compare
jonathan-waarneming-nl
force-pushed
the
feature/new-component-library
branch
from
af7b736
to
537f992
Compare
|
Alles rondom vertalingen kan nu toch weg? |
| import React, { useState } from 'react' | ||
| import { View } from 'react-native' | ||
|
|
||
| // @ts-ignore |
There was a problem hiding this comment.
Deze moest toegevoegd worden omdat het problemen gaf bij het runnen van tsc binnen de observation-app.
| @@ -0,0 +1,3 @@ | |||
| module.exports = { | |||
| presets: ['module:metro-react-native-babel-preset'], | |||
There was a problem hiding this comment.
De 4 plugins die wel worden gebruikt bij ObsIdentify, konden hier weg.
| * we override the overflow property with our own overflow type | ||
| */ | ||
| type FontStyle = TextStyle & { overflow?: 'visible' | 'hidden' | undefined } & { | ||
| [k in MixedSizeCSSPropertiesKeys]?: number | string |
There was a problem hiding this comment.
Hier voeg ik [k in MixedSizeCSSPropertiesKeys]?: number | string toe zodat de typing weer klopt voor h1, h2 en h3 typing in src/styles/html.ts. Die was veranderd door DimensionValue.
| disabled={button!.disabled || false} | ||
| secondary={button!.secondary || false} | ||
| title={button!.title} | ||
| key={i} |
There was a problem hiding this comment.
De key was eerst de title, maar in de unit test "Message - Rendering - Without title" kregen we deze foutmelding:
Warning: Each child in a list should have a unique key prop.
There was a problem hiding this comment.
Normaal gesproken bad practice. De key moet corresponderen met het item zelf, niet met de plek van het item in de lijst. Maar als je identieke LargeButtonProps[] kan doorgeven, dan heb je geen info om een unieke key te maken.
There was a problem hiding this comment.
In https://www.reactnative.express/react/lists_and_keys:
This silences the warning from React about forgetting to include keys, but remember that doing this will cause React to identify elements incorrectly if the data is modified: for example, if a new item is inserted at the front of the list, it will get key '0', which previously belonged to another item. So you may be better off assigning identifiers or indexes to your data set if you can.
Good to know!
|
Na deze PR hebben we nog 2 user stories nodig, maar die zijn nog niet urgent:
|
There was a problem hiding this comment.
Mooi zo.
Het lijkt me goed om na de merge naar develop ook een merge naar master te doen, dan master een tag te geven met het versienummer, en dan in de observation deze tag toe te voegen aan de URL in package.json. Op die manier kunnen we voor de Observation app upgrades van de component-library doen zonder ObsIdentify te raken, en andersom.
| disabled={button!.disabled || false} | ||
| secondary={button!.secondary || false} | ||
| title={button!.title} | ||
| key={i} |
There was a problem hiding this comment.
Normaal gesproken bad practice. De key moet corresponderen met het item zelf, niet met de plek van het item in de lijst. Maar als je identieke LargeButtonProps[] kan doorgeven, dan heb je geen info om een unieke key te maken.
Ik heb https://github.com/observation/obsidentify/issues/858 en https://github.com/observation/app/issues/20 aangemaakt Tevens tag v1.0.0 aangemaakt die (volgens mij) naar de laatste commit in master leidt. |
Create a component library, so we can reuse components and maintain consistency across ObsIdentify and the Observation app.
Resolves: https://github.com/observation/app/issues/7