fix: adapt share process with new stack #653

enguerran · 2017-12-22T13:20:54Z

Multiple changes to feat the new cozy-stack share API.

enguerran · 2017-12-22T13:25:56Z

(5) ETQ utilisateur je peux partager à un ou plusieurs contacts un dossier ou un fichier ou un album photos

y-lohse · 2017-12-22T13:28:59Z

src/sharing/components/ShareByEmail.jsx

-        this.reset()
-      })
+    this.props.onSend(recipients, sharingType)
+    this.reset()


Since you removed the catch here, isn't there a chance that this.props.onSend results in an exception and stops the execution?

Well seen.
If fact removing catch is not the only thing I removed. props.onSend is not a promise anymore. It executes share from whom I removed the return as it did not return anything but where used to return a empty promise that let this very function sendSharingLink executing this.reset() on then or on catch.

As reseting the input field is not related to the resolve of the promise (then or catch) and as the animation is not critical here, I simplify the code:

we execute onSend by passing recipients and sharingType

we execute the asynchronous fetch that creates the sharing

we reset the field

once the sharing fetch is terminated, the ShareByEmailWrapper will alert an info or an error.

Note: I will maybe merge those two components as I don't understand which one is responsible of what.

y-lohse · 2017-12-22T13:29:26Z

So far so good!

enguerran · 2017-12-27T17:17:36Z

New share modal with no tabs but 2 sections:

Autosuggest contacts

Recipient chip

Special case 1

Special case 2

Special case 2 for file

enguerran · 2017-12-28T09:05:50Z

Awaiting cozy/cozy-stack#1130

enguerran · 2017-12-28T11:47:05Z

Un autre visuel quand on a beaucoup de destinataire

enguerran

I will not request changes strictly speaking, but it will be very nice to take those below comments into account.

enguerran · 2018-01-08T14:59:02Z

src/drive/locales/en.json

@@ -55,7 +55,7 @@
      "revoked": "Error"
    },
    "type": {
-      "one-way": "Can View",
+      "one-way": "Can View (coming soon)",


@y-lohse don't forget to update https://www.transifex.com/cozy/cozy-drive/translate/#fr/global/130398143?key=one-way

enguerran · 2018-01-08T15:03:06Z

src/sharing/share.styl

@@ -11,6 +11,9 @@
        cursor pointer

 :local
+    .share-modal
+        max-width 33rem !important // waiting for modal size presets in cozy-ui


That is not really clean.
I discussed with people about this and I told them that it is not third-app risponsibility.

There is a reason we build an UI library.
If this is important, then we should release a brand new version of the library with "size presets".

That said, the commit message is really fun 🍆

enguerran · 2018-01-08T15:05:27Z

src/sharing/share.styl

+        padding 1rem 0
+
+        select
+            flex 0 0 49%


I am not sure 49% is the best way to add space between two elements.

enguerran · 2018-01-08T15:08:58Z

src/sharing/components/ShareByEmail.jsx

@@ -1,6 +1,5 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
-import classnames from 'classnames'

 import { Button } from 'cozy-ui/react'


Must be git squashed or git fixed with the commit that inserts the lint error.

enguerran · 2018-01-08T15:09:35Z

src/lib/cozy-client/slices/sharings.js

@@ -397,7 +397,7 @@ const getDocumentActiveSharings = (state, doctype, id) => {
      perm => perm.attributes.permissions.files.values.indexOf(id) !== -1
    ),
    ...perms.withMe.filter(
-      perm => perm.attributes.permissions.files.values.indexOf(id) !== -1
+      perm => perm.attributes.permissions.rule0.values.indexOf(id) !== -1


rule0 is a very bad name we should change.

I cannot understand if something is wrong when I read on line 397 perm.attributes.permissions.files.values and 3 lines after (line 400) in the exact same scope perm.attributes.permissions.rule0.values.

Same appears on line 412 p.attributes.permissions.rule0 with line 411 p.attributes.permissions.files and line 413 p.attributes.permissions[type]. It is very confusing.

enguerran · 2018-01-08T15:19:21Z

src/sharing/components/ShareByEmail.jsx

+    // even with the `shippedProposals` option to babel-preset-env
+    // .finally(() => {
+    //   this.reset()
+    // })


So we should remove a43e28e#diff-e56633f72ecc521128b3db6586074d2cR9.

enguerran · 2018-01-08T15:21:11Z

src/sharing/components/ShareAutosuggest.jsx

+      event.key === 'Enter' ||
+      (event.keyCode === 13 && this.state.inputValue !== '') ||
+      (event.key === 'Space' ||
+        (event.keyCode === 32 && /^.+@.+/.test(this.state.inputValue)))


We shall add , too as it is an enumeration.

enguerran · 2018-01-08T15:22:12Z

src/sharing/components/ShareAutosuggest.jsx

-            />
+            >
+              <Icon icon="cross" width="16" height="16" />
+            </button>


It's better this way. Thanks.

enguerran · 2018-01-08T15:23:53Z

src/sharing/components/ShareAutosuggest.jsx

+      ((event.key === 'Enter' || event.keyCode === 13) &&
+        this.state.inputValue !== '') ||
+      ((event.key === 'Space' || event.keyCode === 32) &&
+        /^.+@.+/.test(this.state.inputValue))


It shall be fixed or squashed with the previous commit, if not in the really first commit that implies this file.

enguerran · 2018-01-08T15:24:48Z

src/sharing/components/ShareByEmail.jsx

            email: recipients[0].id
              ? getPrimaryEmail(recipients[0])
              : recipients[0].email
          })
        } else {
-          Alerter.info(`${documentType}.share.shareByEmail.genericSuccess`, {
+          Alerter.success(`${documentType}.share.shareByEmail.genericSuccess`, {


Should be squashed or fixed in the previous commit from this PR that introduces the alerter.

but still need a real implementation

like it is done on #652

generated from commit f2a77e5

enguerran changed the title ~~fix: adapt share process with new stack~~ [WI] fix: adapt share process with new stack Dec 22, 2017

enguerran changed the title ~~[WI] fix: adapt share process with new stack~~ [WIP] fix: adapt share process with new stack Dec 22, 2017

enguerran requested review from goldoraf and y-lohse December 22, 2017 13:21

enguerran added the work in progress label Dec 22, 2017

y-lohse reviewed Dec 22, 2017

View reviewed changes

enguerran commented Jan 8, 2018

View reviewed changes

enguerran and others added 13 commits January 9, 2018 10:03

fix: adapt share process with new stack

8671079

feat: new style for share modal

f32a705

refactor: ShareByEmail

04d179c

feat: add sharing special case

5746f67

but still need a real implementation

feat: disable one-way sharing

b858676

feat: add a spinner on submit action

ee1b32e

chore: transform em to rem

e22b7f0

like it is done on #652

style: review from claire et gooz

8958241

feat: enlarge sharing modal

12362bd

fix: adapt sharing modal for mobile

ce7e737

fix: minor style updates

82fb396

feat: add emails on space in autocomplete

3a3ea3b

feat: change alert style

5163ba3

enguerran changed the title ~~[WIP] fix: adapt share process with new stack~~ fix: adapt share process with new stack Jan 9, 2018

enguerran removed the work in progress label Jan 9, 2018

enguerran merged commit f2a77e5 into cozy:master Jan 9, 2018

enguerran deleted the feat.share branch January 9, 2018 09:29

cozycloud pushed a commit that referenced this pull request Jan 9, 2018

publish: Merge pull request #653 from enguerran/feat.share

545d6c9

generated from commit f2a77e5

enguerran mentioned this pull request Jan 10, 2018

fix: sharing infos #679

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: adapt share process with new stack #653

fix: adapt share process with new stack #653

enguerran commented Dec 22, 2017 •

edited

Loading

enguerran commented Dec 22, 2017

y-lohse Dec 22, 2017

enguerran Dec 22, 2017

y-lohse commented Dec 22, 2017

enguerran commented Dec 27, 2017

enguerran commented Dec 28, 2017

enguerran commented Dec 28, 2017 •

edited

Loading

enguerran left a comment

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

enguerran Jan 8, 2018

fix: adapt share process with new stack #653

fix: adapt share process with new stack #653

Conversation

enguerran commented Dec 22, 2017 • edited Loading

enguerran commented Dec 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

y-lohse commented Dec 22, 2017

enguerran commented Dec 27, 2017

New share modal with no tabs but 2 sections:

Autosuggest contacts

Recipient chip

Special case 1

Special case 2

Special case 2 for file

enguerran commented Dec 28, 2017

enguerran commented Dec 28, 2017 • edited Loading

enguerran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enguerran commented Dec 22, 2017 •

edited

Loading

enguerran commented Dec 28, 2017 •

edited

Loading