Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments AutoCompletion #6982

Merged
merged 33 commits into from
Nov 1, 2017
Merged

Comments AutoCompletion #6982

merged 33 commits into from
Nov 1, 2017

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Oct 27, 2017

Follow up to #6779, both backend and gui, for #2443

I also did a quick demo video demo video

  • Type @ and a char to start auto completion for a user
  • Mentiones are rendered just like as we use avatar + displayname everywhere
  • works also when editing users
  • user shared with and commented are sorted on top (given their are part of the result set)
  • it's themed!
  • known issue on FF: to remove a mention, it must be marked first. Apparently a browser bug. On Chromium, backspace works fine.

Please test and review

* introduce a Controller for requests
* introduce result sorting mechanism
* extend Comments to retrieve commentors (actors) in a tree
* add commenters sorter
* add share recipients sorter

Signed-off-by: Arthur Schiwon <[email protected]>
so walking it is reusable

Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
we will need it for nice formatting of the mentioned user

Signed-off-by: Arthur Schiwon <[email protected]>
i.e. avatar with displaymenu and contactsmenu-popover

Signed-off-by: Arthur Schiwon <[email protected]>
also ensures proper rendering, even of edited comments

Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
@codecov
Copy link

codecov bot commented Oct 27, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@52b679a). Click here to learn what that means.
The diff coverage is 58.15%.

@@            Coverage Diff            @@
##             master    #6982   +/-   ##
=========================================
  Coverage          ?   50.62%           
  Complexity        ?    24289           
=========================================
  Files             ?     1576           
  Lines             ?    92882           
  Branches          ?     1358           
=========================================
  Hits              ?    47022           
  Misses            ?    45860           
  Partials          ?        0
Impacted Files Coverage Δ Complexity Δ
lib/private/Share20/Manager.php 86.65% <ø> (ø) 234 <0> (?)
core/routes.php 0% <ø> (ø) 0 <0> (?)
apps/user_ldap/lib/Connection.php 57.55% <ø> (ø) 118 <0> (?)
...iles_sharing/composer/composer/autoload_static.php 0% <ø> (ø) 1 <0> (?)
config/config.sample.php 0% <ø> (ø) 0 <0> (?)
...pps/comments/composer/composer/autoload_static.php 0% <ø> (ø) 1 <0> (?)
apps/user_ldap/lib/Access.php 18.18% <0%> (ø) 304 <0> (?)
...s/comments/composer/composer/autoload_classmap.php 0% <0%> (ø) 0 <0> (?)
lib/private/legacy/app.php 54.18% <0%> (ø) 221 <0> (?)
apps/comments/appinfo/app.php 13.63% <0%> (ø) 0 <0> (?)
... and 11 more

Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
Signed-off-by: Arthur Schiwon <[email protected]>
background: we have a flat hierarchy of comments, not a tree. therefore we
can also remove again the unnecessary additions.

Signed-off-by: Arthur Schiwon <[email protected]>
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks nice from the demo video! I’d say further fixes should be done in follow-ups cause this is too big already. :)

@nickvergessen
Copy link
Member

Should only ship the minified js file, instead of the examples and other unneeded stuff.

@@ -1011,6 +1011,11 @@ public function __construct($webRoot, \OC\Config $config) {
});
$this->registerAlias('CollaboratorSearch', \OCP\Collaboration\Collaborators\ISearch::class);

$this->registerService(\OCP\Collaboration\AutoComplete\IManager::class, function (Server $c) {
return new Collaboration\AutoComplete\Manager($c);
Copy link
Member

Choose a reason for hiding this comment

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

please import completely

Copy link
Member

Choose a reason for hiding this comment

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

actually you should be able to drop this completely because of our magic automatic injection

@nickvergessen
Copy link
Member

  1. If you match the full user id with @test1 the user is not suggested anymore, but @test brings them up
  2. The avatar is not rendered anymore after you clicked/selected one of the options.

@blizzz
Copy link
Member Author

blizzz commented Oct 31, 2017

@nickvergessen great testing 👍 fixed with recent commits

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Awesome

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz merged commit db48575 into master Nov 1, 2017
@blizzz blizzz deleted the autocomplete-gui branch November 1, 2017 16:00
@blizzz
Copy link
Member Author

blizzz commented Nov 1, 2017

🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants