Skip to content

Comments

Add quick actions for contacts#3401

Merged
GretaD merged 1 commit intomainfrom
enhanc/quick-actions
Jul 31, 2023
Merged

Add quick actions for contacts#3401
GretaD merged 1 commit intomainfrom
enhanc/quick-actions

Conversation

@GretaD
Copy link
Contributor

@GretaD GretaD commented May 23, 2023 •
edited
Loading

Screenshot from 2023-06-27 16-04-40
Screenshot from 2023-06-27 16-04-28

fixes #3365

@GretaD GretaD added enhancement New feature or request 2. developing Work in progress labels May 23, 2023
@GretaD GretaD self-assigned this May 23, 2023
@codecov
Copy link

codecov bot commented May 23, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: +0.24% 🎉

Comparison is base (dc8bf5a) 1.82% compared to head (8bc1765) 2.07%.
Report is 5 commits behind head on main.

❗ Current head 8bc1765 differs from pull request most recent head 30b1360. Consider uploading reports for the commit 30b1360 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##              main   #3401      +/-   ##
==========================================
+ Coverage     1.82%   2.07%   +0.24%     
==========================================
  Files          112      89      -23     
  Lines         6072    5356     -716     
  Branches      1480    1499      +19     
==========================================
  Hits           111     111              
+ Misses        5845    5127     -718     
- Partials       116     118       +2
Files Changed Coverage Δ
src/components/ContactDetails.vue 0.00% <0.00%> (ø)
src/models/contact.js 12.02% <0.00%> (-0.41%) ⬇️
src/models/rfcProps.js 45.45% <ø> (ø)
src/services/isTalkEnabled.js 0.00% <0.00%> (ø)

... and 24 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GretaD GretaD force-pushed the enhanc/quick-actions branch from 314b8bc to cfb0019 Compare June 2, 2023 08:37
GretaD
GretaD commented Jun 5, 2023
View reviewed changes
GretaD
GretaD commented Jun 5, 2023
View reviewed changes
GretaD
GretaD commented Jun 26, 2023
View reviewed changes
ChristophWurst
ChristophWurst reviewed Jun 27, 2023
View reviewed changes
lib/Controller/PageController.php Outdated
$talkVersion = $this->appManager->getAppVersion('spreed');
$isTalkEnabled = $this->appManager->isEnabledForUser('spreed') === true;

$isTalkVersionCompatible = $this->compareVersion->isCompatible($talkVersion ? $talkVersion : '0.0.0', 15);
Copy link
Member

@ChristophWurst ChristophWurst Jun 27, 2023

Choose a reason for hiding this comment

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

is 15 arbitrary or did the talk links not work before that?

Copy link
Contributor Author

@GretaD GretaD Jun 27, 2023

Choose a reason for hiding this comment

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

no its not arbitrary, i quickly checked releases and i didnt see backports on 14th i think, but i will ask Marco or Joas for a clear answer

Copy link
Member

@ChristophWurst ChristophWurst Jun 27, 2023

Choose a reason for hiding this comment

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

the feature was added with nextcloud/spreed#300, so v2

ChristophWurst
ChristophWurst reviewed Jun 27, 2023
View reviewed changes
ChristophWurst
ChristophWurst reviewed Jun 27, 2023
View reviewed changes
@ChristophWurst
Copy link
Member

ChristophWurst commented Jun 27, 2023 •
edited by GretaD
Loading

  • BUG: [Vue warn]: Error in render: "TypeError: this.phoneNumberProperty.getFirstValue is not a function" for contacts with no phone number

@ChristophWurst
Copy link
Member

ChristophWurst commented Jun 27, 2023 •
edited
Loading

  • BUG: Quick actions are not aligned with the rest of the page

image

Edit: they are placed properly for Contacts with Detailed name, not for those without

@GretaD
Copy link
Contributor Author

GretaD commented Jun 29, 2023

  • BUG: [Vue warn]: Error in render: "TypeError: this.phoneNumberProperty.getFirstValue is not a function" for contacts with no phone number

i cannot see this in my logs. I see that the icons dont have background color, are you sure you have the latest changes?

ChristophWurst
ChristophWurst reviewed Jun 29, 2023
View reviewed changes
ChristophWurst
ChristophWurst reviewed Jun 29, 2023
View reviewed changes
@JohannesGGE
Copy link
Contributor

JohannesGGE commented Jun 30, 2023

I added a fixup with the access logic to the profileURL

@GretaD
Copy link
Contributor Author

GretaD commented Jun 30, 2023

I added a fixup with the access logic to the profileURL

thank you Johannes, i will have a look asap

@GretaD GretaD force-pushed the enhanc/quick-actions branch from 6b84623 to 6d3470b Compare July 18, 2023 16:25
st3iny
st3iny reviewed Jul 25, 2023
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

I tested it briefly. Design still needs some work as the buttons are aligned incorrectly.

st3iny
st3iny reviewed Jul 28, 2023
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment •
edited by GretaD
Loading

Choose a reason for hiding this comment

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

Follow up:

  • ### Empty email addresses are not filtered

Note: The contact has empty email address fields. This is not common in practice but still looks weird.

grafik

  • ### Quick actions take a lot of horizontal space and don't wrap

Might not be related and can be fixed later.

grafik

  • ### Clicking the talk button does nothing Fixed

There is no error in the console.

grafik

@GretaD GretaD requested review from kesselb and st3iny July 28, 2023 15:39
@GretaD GretaD marked this pull request as ready for review July 28, 2023 16:20
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 28, 2023
st3iny
st3iny approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works. The design needs some attention in a followup.

@GretaD GretaD force-pushed the enhanc/quick-actions branch from e3598e1 to 30b1360 Compare July 31, 2023 11:09
@GretaD GretaD enabled auto-merge July 31, 2023 11:09
@GretaD GretaD merged commit c77ff78 into main Jul 31, 2023
@GretaD GretaD deleted the enhanc/quick-actions branch July 31, 2023 11:12
@GretaD GretaD mentioned this pull request Jul 31, 2023
Closed
4 tasks
@ChristophWurst ChristophWurst mentioned this pull request Aug 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurst ChristophWurst ChristophWurst left review comments

@st3iny st3iny st3iny approved these changes

@kesselb kesselb Awaiting requested review from kesselb

Assignees

@GretaD GretaD

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add quick actions for each contact

5 participants

@GretaD @ChristophWurst @JohannesGGE @st3iny @kesselb