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

Enh/add virtual group #1687

Merged
merged 9 commits into from
Aug 21, 2020
Merged

Enh/add virtual group #1687

merged 9 commits into from
Aug 21, 2020

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 4, 2020

Peek 04-07-2020 10-07
outdated gif


  • Virtual group & Add group button
  • Base UI
  • Dav plugin
  • Picker
    • Cleanup & fix UserBubble component
    • Standardisation
    • QA
  • Progress
    • Modal
    • Animation
    • Finish state
    • QA
  • Update local contacts
  • QA

Bugs

  • Do NOT provide read-only contacts
  • Do NOT provide contacts that already belong to the group

@skjnldsv skjnldsv added 2. developing Work in progress enhancement New feature or request high High priority performance Performance issues and optimisations labels Jul 4, 2020
@skjnldsv skjnldsv self-assigned this Jul 4, 2020
@skjnldsv skjnldsv added this to the next minor milestone Jul 4, 2020
@codecov
Copy link

codecov bot commented Jul 4, 2020

Codecov Report

Merging #1687 into master will decrease coverage by 12.45%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master    #1687       +/-   ##
=============================================
- Coverage     40.05%   27.60%   -12.46%     
- Complexity      128      146       +18     
=============================================
  Files            17       18        +1     
  Lines           377      442       +65     
=============================================
- Hits            151      122       -29     
- Misses          226      320       +94     
Impacted Files Coverage Δ Complexity Δ
appinfo/app.php 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
lib/AppInfo/Application.php 0.00% <0.00%> (ø) 3.00 <3.00> (+2.00)
lib/Controller/PageController.php 0.00% <0.00%> (-100.00%) 3.00 <0.00> (ø)
lib/Dav/PatchPlugin.php 0.00% <0.00%> (ø) 16.00 <16.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3ae90c...d3f8837. Read the comment docs.

@skjnldsv skjnldsv requested review from jancborchardt and oparoz July 4, 2020 08:08
@skjnldsv skjnldsv force-pushed the enh/add-virtual-group branch 2 times, most recently from b0c8d00 to eefe767 Compare July 14, 2020 07:33
@jancborchardt

This comment has been minimized.

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.

Some more review:

  • The "New group" entry should be in var(--color-text-maxcontrast)
  • When clicking the "New group" entry, the input field in the popdown could be focused so people can directly start typing
  • Label of the "Add some" button in the group emptycontent would be better "Add contacts"
  • After successful adding, the "Close" button is weirdly positioned right of the progress bar. Better would be below it, centered
    image
  • As said before: If there are no errors on adding, that modal can automatically be closed after ~3 seconds of showing the "Added …" feedback view.
  • When there is only 1 or few contacts, the "Add some" modal vertically centers that contact (issue in the component I guess)
    image
  • You can add new groups, but there is no way to delete them
  • Empty groups are removed on refresh. I guess this is to keep in line with spec? Just a bit confusing, but not so important
  • If the counter is 0, the counter does not need to be shown. That’s a general thing true everywhere, and we can probably put that logic directly in the component?
  • For some reason, I do get the "1 contact added to group" message, but the contact doesn’t seem to be actually added: the counter doesn’t go up, and the contact doesn’t show in the group? Getting this in the console:
Error: Request failed with status code 501
    createError createError.js:16
    settle settle.js:17
    handleLoad xhr.js:61
Contacts.vue:801
    _callee3$/</< Contacts.vue:801

@jancborchardt
Copy link
Member

(Btw I’m also getting this error of "This contact was broken and received a fix …" on a completely new installations with a contact I just created with just a name. Anything I can do to fix that, or is there a pull request?)

@skjnldsv
Copy link
Member Author

(Btw I’m also getting this error of "This contact was broken and received a fix …" on a completely new installations with a contact I just created with just a name. Anything I can do to fix that, or is there a pull request?)

Not related to this. No pr yet

@skjnldsv skjnldsv mentioned this pull request Aug 9, 2020
@skjnldsv skjnldsv linked an issue Aug 14, 2020 that may be closed by this pull request
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the enh/add-virtual-group branch from fa81e8f to c179416 Compare August 21, 2020 08:02
…eady selected ones

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the enh/add-virtual-group branch from c179416 to d3f8837 Compare August 21, 2020 08:05
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Aug 21, 2020
@skjnldsv
Copy link
Member Author

@jancborchardt let me merge and fix in followup so it's easier for me

@skjnldsv skjnldsv merged commit 87615fb into master Aug 21, 2020
@skjnldsv skjnldsv deleted the enh/add-virtual-group branch August 21, 2020 08:08
@skjnldsv skjnldsv modified the milestones: next minor, next Sep 6, 2020
@skjnldsv skjnldsv modified the milestones: next, 3.4.0 Oct 1, 2020
@kesselb kesselb mentioned this pull request Jul 21, 2021
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request high High priority performance Performance issues and optimisations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a group from the group/left pane
2 participants