Skip to content

Comments

fix(contacts): display name not uri for recent-contacted address book#3339

Merged
ChristophWurst merged 1 commit intomainfrom
fix/2187/raw-recent-contacted-uri-visible
Apr 26, 2023
Merged

fix(contacts): display name not uri for recent-contacted address book#3339
ChristophWurst merged 1 commit intomainfrom
fix/2187/raw-recent-contacted-uri-visible

Conversation

@JohannesGGE
Copy link
Contributor

Fix: #2187

before after
Screenshot from 2023-04-26 13-39-04 Screenshot from 2023-04-26 13-39-46
Screenshot from 2023-04-26 13-39-17 Screenshot from 2023-04-26 13-40-03

@JohannesGGE JohannesGGE added bug Something isn't working 3. to review Waiting for reviews labels Apr 26, 2023
@JohannesGGE JohannesGGE self-assigned this Apr 26, 2023
Comment on lines 101 to 103
return this.options.
filter(option => option.readOnly === undefined || !option.readOnly).
map(addressbook => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return this.options.
filter(option => option.readOnly === undefined || !option.readOnly).
map(addressbook => {
return this.options
.filter(option => option.readOnly === undefined || !option.readOnly).
.map(addressbook => {

@ChristophWurst ChristophWurst requested a review from st3iny April 26, 2023 11:58
@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.87 ⚠️

Comparison is base (713fe88) 1.86% compared to head (f1c8c06) 0.00%.

❗ Current head f1c8c06 differs from pull request most recent head 6421bd4. Consider uploading reports for the commit 6421bd4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##              main   #3339      +/-   ##
==========================================
- Coverage     1.86%   0.00%   -1.87%     
  Complexity     272     272              
==========================================
  Files          112      25      -87     
  Lines         5963     824    -5139     
  Branches      1401       0    -1401     
==========================================
- Hits           111       0     -111     
+ Misses        5738     824    -4914     
+ Partials       114       0     -114     

see 87 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ChristophWurst
Copy link
Member

ChristophWurst commented Apr 26, 2023

  • BUG: the addressbook prop is no longer read-only. I can change it, but run into an error

main

Bildschirmfoto vom 2023-04-26 14-02-34

fix/2187/raw-recent-contacted-uri-visible default state

Bildschirmfoto vom 2023-04-26 14-02-54

fix/2187/raw-recent-contacted-uri-visible focus state

Bildschirmfoto vom 2023-04-26 14-02-57

@JohannesGGE JohannesGGE added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 26, 2023
@JohannesGGE
Copy link
Contributor Author

@ChristophWurst This seems to be a generel bug with the PropertySelect component because the isReadOnly is not passed, so it's always false.
I fixed it anyway 👍

@JohannesGGE JohannesGGE added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 26, 2023
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.

*/
selectableOptions() {
return this.options
.filter(option => option.readOnly === undefined || !option.readOnly)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.filter(option => option.readOnly === undefined || !option.readOnly)
.filter(option => !option.readOnly)

Could be simplified.

Comment on lines 497 to 506
addressbooksOptions() {
return this.addressbooks
.filter(addressbook => !addressbook.readOnly && addressbook.enabled)
.filter(addressbook => addressbook.enabled)
.map(addressbook => {
return {
id: addressbook.id,
name: addressbook.displayName,
readOnly: addressbook.readOnly,
}
})
Copy link
Member

Choose a reason for hiding this comment

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

This could have side effects. However, I did not spot any during my testing and the address book options made sense.

@JohannesGGE JohannesGGE force-pushed the fix/2187/raw-recent-contacted-uri-visible branch from 9e75e67 to 6421bd4 Compare April 26, 2023 13:21
@ChristophWurst ChristophWurst merged commit 99987ce into main Apr 26, 2023
@ChristophWurst ChristophWurst deleted the fix/2187/raw-recent-contacted-uri-visible branch April 26, 2023 13:30
@JohannesGGE
Copy link
Contributor Author

/backport to stable5.2

@JohannesGGE
Copy link
Contributor Author

/backport to stable4.2

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

Labels

3. to review Waiting for reviews bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raw z-app-generated--contactsinteraction--recent URI visible to user

3 participants