Skip to content

Dimensions πŸ“ - updated Docs πŸ“‘ & new snack examples ✨#1601

Closed
DaniAkash wants to merge 1 commit intofacebook:masterfrom
DaniAkash:dimensions-snack
Closed

Dimensions πŸ“ - updated Docs πŸ“‘ & new snack examples ✨#1601
DaniAkash wants to merge 1 commit intofacebook:masterfrom
DaniAkash:dimensions-snack

Conversation

@DaniAkash
Copy link
Copy Markdown
Contributor

@DaniAkash DaniAkash commented Feb 6, 2020

Belongs to #1579

Inspiration - To address this stackoverflow question

Contains the snack with both class & functional component examples and also a bit of update to the documentation regarding screen, window & event listener.

Pending items in this PR:

  • useDimensions hook
  • Info regarding scaling
  • Verify if rounding is necessary for using Dimensions

@react-native-bot
Copy link
Copy Markdown

Deploy preview for react-native ready!

Built with commit 4744b09

https://deploy-preview-1601--react-native.netlify.com

Changes to docs/ are reflected in the next "master" version.

Thank you for your contributions.

How to Contribute β€’ Documentation Sources


### React Native Hooks

You can also try [useDimensions](https://github.com/react-native-community/react-native-hooks#usedimensions) hook from [React native hooks](https://github.com/react-native-community/react-native-hooks) library which makes handling screen/window size changes much simpler.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RN actually ships its own useWindowDimensions hook now:
https://github.com/facebook/react-native/blob/8d57691a606ce81e6d861f93ccc11875088f58b6/Libraries/Utilities/useWindowDimensions.js

The docs should probably describe that hook here instead

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I didn't knew!

| dim | string | Yes | Name of dimension as defined when calling `set`. @returns {Object?} Value for the dimension. |
| Name | Type | Required | Description |
| ---- | ------ | -------- | -------------------------------------------------------------------------------------------- |
| dim | string | Yes | Name of dimension as defined when calling `set`. @returns {Object?} Value for the dimension. |
Copy link
Copy Markdown
Contributor

@jeremy-deutsch jeremy-deutsch Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| dim | string | Yes | Name of dimension as defined when calling `set`. @returns {Object?} Value for the dimension. |
| dim | string | Yes | Name of dimension (`"window"` or `"screen"`). |
Returns an object with four number fields: `width`, `height`, `scale`, and `fontScale`.

It seems weird to me to reference set from the documentation for get, and the @returns is just a copy-paste from the code. scale and numberScale should maybe be documented better here, but honestly, I don't really know much about them!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, these areas could use some more detail. I'll try them out and update the docs

@DaniAkash DaniAkash mentioned this pull request Feb 7, 2020
@Simek
Copy link
Copy Markdown
Collaborator

Simek commented Feb 7, 2020

In the first example documentation is suggesting rounding Dimensions values before usage. This part was added in #1274. I don't know the exact reason behind this but I would assume that in some cases/on some devices this value could return float.

Currently proposed examples do not utilize Math.round. I'm not sure which approach to handling those values is better. But overall I think that one of maintainers should point out recommended approach for the documentation consistency. After that some parts of this PR should be updated accordingly.

@DaniAkash
Copy link
Copy Markdown
Contributor Author

Closing it in favour of #1671

@DaniAkash DaniAkash closed this Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants