Add more personal information fields to the settings page for enhanced federated sharing

[schiessle]

Continue to develop the advanced personal settings where people can add more personal information and publish them if they wish for internal use, public or for federated sharing

This is the PR @ChristophWurst and I worked on a few months ago. I just rebased it and had to resolve many merge conflicts. Also some UX part is broken now. 😢

• fix avatar upload
• allow to crop the avatar
• drop down menus are not allign correctly
• spinner next to "profile picture" doesn't disappear
• drop down for avatar scope is broken
• Don't fail when email is empty

@ChristophWurst do you have some time to have a look at this front-end issues, so that I can concentrate on the back-end stuff? This would be great!

[mention-bot]

@schiessle, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MorrisJobke, @jancborchardt and @icewind1991 to be potential reviewers.

[ChristophWurst]

@jancborchardt @skjnldsv any idea how I could structure and style the HTML to show the popover menu underneath the icon in the header?

Currently it looks like this

<div class="personal-settings-setting-box">
        <form id="emailform" class="section">
            <h2>
                <label for="email">Email</label>
                <span class="icon-password">
            </span><div class="federationScopeMenu popovermenu bubble hidden open menu" style="right: 122px;"></div></h2>
            <input name="email" id="email" value="[email protected]" placeholder="Your email address" autocomplete="on" autocapitalize="off" autocorrect="off" type="email">
            <input id="emailscope" value="private" type="hidden">
            <br>
            <em>For password recovery and notifications</em>
            <span class="icon-checkmark hidden">

    </span></form></div>

where I position it absolute and calculate the distance to the right side with js. That apparently does or does not work depending on the width of the header label (e.g. "Email" is shorter than "Phone number").

[ChristophWurst]

• TODO: fix change password layout
  

[ChristophWurst]

@jancborchardt I did not manage to fix it, even though you gave me some really good pointers.

My approach was

diff --git a/settings/css/settings.css b/settings/css/settings.css
index 19487d4..b35f1c5 100644
--- a/settings/css/settings.css
+++ b/settings/css/settings.css
@@ -86,9 +86,10 @@ input#openid, input#webdav { width:20em; }
 #personal-settings-container > div h2 {
    position: relative;
 }
-#personal-settings-container > div h2 span[class^="icon-"],
-#personal-settings-avatar-container h2 span[class^="icon-"] {
+#personal-settings-container > div span[class^="icon-"],
+#personal-settings-avatar-container span[class^="icon-"] {
    display: inline-block;
+   position: relative;
    margin-left: 5px;
    background-size: 110%;
    opacity: 0.3;
@@ -99,6 +100,9 @@ input#openid, input#webdav { width:20em; }
 .personal-settings-setting-box input[type="tel"] {
    width: 17em;
 }
+.personal-settings-setting-box h2 {
+   display: inline-block;
+}
 #personal-settings-container > div > form span[class^="icon-checkmark"] {
    position: absolute;
    left: 239px;
diff --git a/settings/js/federationsettingsview.js b/settings/js/federationsettingsview.js
index 73b9f85..dc21768 100644
--- a/settings/js/federationsettingsview.js
+++ b/settings/js/federationsettingsview.js
@@ -62,7 +62,7 @@
            var self = this;
            _.each(this._inputFields, function(field) {
                var $heading = self.$('#' + field + 'form h2');
-               var $icon = self.$('#' + field + 'form h2 > span');
+               var $icon = self.$('#' + field + 'form > span');
                var scopeMenu = new OC.Settings.FederationScopeMenu();

                self.listenTo(scopeMenu, 'select:scope', function(scope) {
@@ -71,12 +71,6 @@
                $heading.append(scopeMenu.$el);
                $icon.on('click', _.bind(scopeMenu.show, scopeMenu));

-               // Fix absolute position according to the heading text length
-               // TODO: find alternative to those magic number
-               var diff = field === 'avatar' ? 104 : 68;
-               var pos = ($heading.width() - $heading.find('label').width()) - diff;
-               scopeMenu.$el.css('right', pos);
-
                // Restore initial state
                self._setFieldScopeIcon(field, self._config.get(field + 'Scope'));
            });
diff --git a/settings/templates/personal.php b/settings/templates/personal.php
index 432b05a..40572fc 100644
--- a/settings/templates/personal.php
+++ b/settings/templates/personal.php
@@ -71,13 +71,15 @@ if($_['displayNameChangeSupported']) {
        <form id="displaynameform" class="section">
            <h2>
                <label for="displayname"><?php p($l->t('Full name')); ?></label>
-               <span class="icon-password"/>
            </h2>
-           <input type="text" id="displayname" name="displayname"
-                  value="<?php p($_['displayName']) ?>"
-                  autocomplete="on" autocapitalize="off" autocorrect="off" />
-           <span class="icon-checkmark hidden"/>
-           <input type="hidden" id="displaynamescope" value="<?php p($_['displayNameScope']) ?>">
+           <span class="icon-password"></span>
+           <div>
+               <input type="text" id="displayname" name="displayname"
+                      value="<?php p($_['displayName']) ?>"
+                      autocomplete="on" autocapitalize="off" autocorrect="off" />
+               <span class="icon-checkmark hidden"/>
+               <input type="hidden" id="displaynamescope" value="<?php p($_['displayNameScope']) ?>">
+           </div>
        </form>
    </div>
    <div class="personal-settings-setting-box">

which works kind of, but the menu's width is then based on the parent's width - the h2. For short heading text this makes the popover menu too narrow :-/

[jancborchardt]

@skjnldsv thanks a lot for your flexbox magic! :)

[skjnldsv]

I hate to do that, but since files use the action class, and because files/css/files.css has a lot of related values, this is the easier way.

[jancborchardt]

@skjnldsv yeah, let it be forgiven ;)

[schiessle]

@skjnldsv looks good, thanks for your help!

[schiessle]

As already said, the menus look great now. But what is still broken is the profile picture upload. @skjnldsv @ChristophWurst maybe you can have a look at it? Thanks!

[skjnldsv]

@schiessle what doesn't work exactly?

[schiessle]

If you upload a profile picture you should get a preview which allows you to select the region you want to use. This works on master but is for some reasons broken on this branch (which is beside our commits here the same as master)

This is how it looks at master:

And this is how it looks on this branch:

[schiessle]

@icewind1991 can you have a look at the avatar upload issue? Thanks!

[schiessle]

@skjnldsv I just saw that the drop-down for the avatar is not positioned correctly, it shows up at the top left. All other drop downs work fine. Maybe you can have another look? Thanks a lot!

[skjnldsv]

It's related tot the js handling the popup, I don't know what is causing this :/

[schiessle]

👍 for all the stuff not done by me... second reviewer... Let's get this in. 😉

PS: It is OK, that not every commit is signed, some of them are already older than the rule

[LukasReschke]

@LukasReschke Just to double check: Did the look up server now stored every test I did with my dev instance? I assume not, because the signature couldn't be verified, right?

Correct.

[LukasReschke]

LGTM

[jancborchardt]

Awesome stuff, good work all around :)

[nickvergessen]

This here is quite crucial, needs to be fixed in a new PR and we should drop all the registerService calls from here again, as mentioned below by Christoph

[LukasReschke]

🙈

[LukasReschke]

=> #2234