Upgrade app structure #41
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
February 25, 2020 17:21
Proposals is a component in the screens folder, separated from the App file, and recieves `proposals` and `selectProposal` as props from the App component.
App logic passes function hooks to whoever calls it, such as `selectProposal` or `isSyncing`.
Major change in the component, that now renders the proposals using the `Proposals` component, and passes functions to its components via the appLogic hook. There were some UI updates such as Header, that is simpler and mobile friendly.
sembrestels
reviewed
Feb 25, 2020
added 16 commits
February 25, 2020 19:55
If there are no proposals made, then the app will render this.
When a proposal is selected, a proposal detail page opens with the details from that proposal.
App now works in a different logic - according to the hooks state, selectedProposal and proposals.
This logic is passed to App and it's child components via props, if necessary.
…onal All components are dummy components, only rendering data that is passed to them.
Components, state and logic are only rendered and passed through here.
sembrestels
reviewed
Feb 27, 2020
| }) | ||
| const [isDisabled, setStatus] = useState(true) | ||
|
|
||
| const isFormValid = form => form.filter(i => i === '' || i === 0).length === 0 |
There was a problem hiding this comment.
We should only consider required fields here.
Comment on lines
38
to
47
| <Field label="Description"> | ||
| <TextInput | ||
| onChange={event => | ||
| setForm({ ...form, description: event.target.value }) | ||
| } | ||
| value={form.description} | ||
| wide | ||
| multiline | ||
| /> | ||
| </Field> |
There was a problem hiding this comment.
We can't store proposal descriptions yet (as they should be stored and pinned on IPFS, and we are not ready for that). I would remove this field for now.
app/src/screens/ProposalDetail.js
Outdated
Comment on lines
106
to
115
| <div> | ||
| <H2 color={theme.surfaceContentSecondary}>Description</H2> | ||
| <Text | ||
| css={` | ||
| ${textStyle('body2')}; | ||
| `} | ||
| > | ||
| {description || DEFAULT_DESCRIPTION} | ||
| </Text> | ||
| </div> |
There was a problem hiding this comment.
Let's not display descriptions for now.
app/src/screens/Proposals.js
Outdated
| ${textStyle('body3')}; | ||
| `} | ||
| > | ||
| {description.slice(0, 29) + '...'} |
There was a problem hiding this comment.
I would not show description in the table, as we want to empathize the title.
vivianedias
force-pushed
the
feature/upgrade-app-structure
branch
from
cb2fcdb
to
7504b8d
Compare
|
Thanks @vivianedias! @PJColombo and me have reviewed it today, and that's fantastic! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update app structure to use hooks and add dark mode feature.
proposals/{$id}, so if this link would be shared later, the user would be redirected to this specific proposal.