Skip to content

feat(xo-web/setting/remotes): ability to change the type of a remote#3207

Merged
pdonias merged 7 commits intomasterfrom
settingsRemotes-childrenComponent
Jul 25, 2018
Merged

feat(xo-web/setting/remotes): ability to change the type of a remote#3207
pdonias merged 7 commits intomasterfrom
settingsRemotes-childrenComponent

Conversation

@badrAZ
Copy link
Copy Markdown
Contributor

@badrAZ badrAZ commented Jul 19, 2018

Fixes #2423

Screenshot

image

Check list

Check items when done or if not relevant

  • PR reference the relevant issue (e.g. Fixes #007)
  • if UI changes, a screenshot has been added to the PR
  • CHANGELOG:
    • enhancement/bug fix entry added
    • list of packages to release updated (${name} v${new version})
  • documentation updated

Process

  1. create a PR as soon as possible
  2. mark it as WiP: (Work in Progress) if not ready to be merged
  3. when you want a review, add a reviewer
  4. if necessary, update your PR, and re- add a reviewer

@badrAZ badrAZ changed the title [WiP] feat(xo-web/settings/remotes): improve remote management feat(xo-web/setting/remotes): ability to change the type of a remote Jul 20, 2018
@badrAZ badrAZ force-pushed the settingsRemotes-childrenComponent branch from a590844 to 0324997 Compare July 20, 2018 07:25
@badrAZ badrAZ self-assigned this Jul 20, 2018
@badrAZ badrAZ force-pushed the settingsRemotes-childrenComponent branch 2 times, most recently from 744832a to 0c51911 Compare July 20, 2018 08:30
@badrAZ badrAZ requested a review from julien-f July 20, 2018 08:33
Comment thread packages/xo-web/src/common/utils.js Outdated
export const toggleState = (_, { target: { name } }) => state => ({
...state,
[name]: !state[name],
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe create a dedicated freactal-utils module?

Also, add a link to the related Freactal issue and a todo to remove them once resolved.

@badrAZ badrAZ requested a review from julien-f July 20, 2018 12:33
@badrAZ badrAZ force-pushed the settingsRemotes-childrenComponent branch from 0c51911 to 8b7fb0b Compare July 20, 2018 12:35
@badrAZ badrAZ requested a review from pdonias July 20, 2018 13:36
@badrAZ badrAZ force-pushed the settingsRemotes-childrenComponent branch from 8b7fb0b to 7bbb2b8 Compare July 20, 2018 14:08
@badrAZ badrAZ force-pushed the settingsRemotes-childrenComponent branch from 18b5393 to 5619373 Compare July 24, 2018 08:37
export const linkState = (_, { target }) => state => ({
...state,
[target.name]:
target.nodeName.toLowerCase() === 'input' &&
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you check this?

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.

IMHO, to handle the case when it's used in a wrapped checkbox input. I just copy/past the @julien-f implementation.

@pdonias pdonias merged commit b95fc86 into master Jul 25, 2018
@pdonias pdonias deleted the settingsRemotes-childrenComponent branch July 25, 2018 12:24
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.

[Remotes] Possibility to change the type of a remote

3 participants