Improve guidelines (#15008)
- New section: Use [] instead of () in Reagent components: New section after this comment https://github.com/status-im/status-mobile/pull/15005#discussion_r1098220176 - Updated section "Don't use percents to define width/height": reworded to follow the style of the rest of the document. - Update section "Accessibility labels": added example about avoiding dynamic labels.
This commit is contained in:
parent
875f9f1acc
commit
0c59ed5ceb
|
@ -63,12 +63,19 @@ the source file. For a real example, see
|
||||||
|
|
||||||
### Don't use percents to define width/height
|
### Don't use percents to define width/height
|
||||||
|
|
||||||
We shouldn't use percentage:
|
In ReactNative, all layouts use the [flexbox
|
||||||
|
model](https://reactnative.dev/docs/flexbox), so percentages are unnecessary the
|
||||||
- because 100% doesn't make sense in flexbox
|
vast majority of the time, don't use them. Check out this great [interactive
|
||||||
- because we always have fixed margins or paddings in the design. For example, Instead of using `80%` we should use `padding-horizontal 20`. This is because `%` will be different on different devices in pixels, but we should always have same the paddings in pixels on all devices.
|
flexbox guide](https://www.joshwcomeau.com/css/interactive-guide-to-flexbox/) by
|
||||||
|
Joshua Comeau.
|
||||||
|
|
||||||
|
```clojure
|
||||||
|
;; bad
|
||||||
|
[rn/view {:style {:width "80%"}}]
|
||||||
|
|
||||||
|
;; good
|
||||||
|
[rn/view {:style {:padding-horizontal 20}}]
|
||||||
|
```
|
||||||
|
|
||||||
#### Styles def vs defn
|
#### Styles def vs defn
|
||||||
|
|
||||||
|
@ -163,6 +170,26 @@ Use the simple `defn` to declare components. Don't use `utils.views/defview` and
|
||||||
(do-something window-width)))
|
(do-something window-width)))
|
||||||
```
|
```
|
||||||
|
|
||||||
|
#### Use `[]` instead of `()` in Reagent components
|
||||||
|
|
||||||
|
- The `()` version [does NOT work with Form-2 and
|
||||||
|
Form-3](https://github.com/reagent-project/reagent/blob/master/doc/UsingSquareBracketsInsteadOfParens.md#a-further-significant-why)
|
||||||
|
components.
|
||||||
|
- Components defined with `[]` will be [more efficient at re-render
|
||||||
|
time](https://github.com/reagent-project/reagent/blob/master/doc/UsingSquareBracketsInsteadOfParens.md#which-and-why)
|
||||||
|
because they're interpreted by Reagent and transformed into distinct React
|
||||||
|
components, with their own lifecycle.
|
||||||
|
|
||||||
|
```clojure
|
||||||
|
;; bad
|
||||||
|
[rn/view
|
||||||
|
(message-card message)]
|
||||||
|
|
||||||
|
;; good
|
||||||
|
[rn/view
|
||||||
|
[message-card message]]
|
||||||
|
```
|
||||||
|
|
||||||
### Using re-frame subscriptions and dispatching events
|
### Using re-frame subscriptions and dispatching events
|
||||||
|
|
||||||
Use the `utils.re-frame` namespace instead of `re-frame.core` to subscribe and
|
Use the `utils.re-frame` namespace instead of `re-frame.core` to subscribe and
|
||||||
|
@ -300,8 +327,8 @@ core interop macros.
|
||||||
|
|
||||||
### Accessibility labels
|
### Accessibility labels
|
||||||
|
|
||||||
Use keywords instead of strings. As a bonus, remember keywords are cached in
|
Accessibility labels are currently used only for end-to-end tests. Use keywords
|
||||||
memory.
|
instead of strings (remember keywords are cached).
|
||||||
|
|
||||||
```clojure
|
```clojure
|
||||||
;; bad
|
;; bad
|
||||||
|
@ -313,6 +340,18 @@ memory.
|
||||||
"Markov"]
|
"Markov"]
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Avoid dynamic labels, for example to specify an element's index because
|
||||||
|
[Appium](https://appium.io/) already supports element selection based on
|
||||||
|
indices.
|
||||||
|
|
||||||
|
```clojure
|
||||||
|
;; bad
|
||||||
|
[button {:accessibility-label (str "do-something" index)}]
|
||||||
|
|
||||||
|
;; good
|
||||||
|
[button {:accessibility-label :do-something}]
|
||||||
|
```
|
||||||
|
|
||||||
### Icons
|
### Icons
|
||||||
|
|
||||||
Use the appropriate keyword qualification/namespace and don't directly require
|
Use the appropriate keyword qualification/namespace and don't directly require
|
||||||
|
|
Loading…
Reference in New Issue