-
Notifications
You must be signed in to change notification settings - Fork 198
fix(ContactsSettings): redo #4803
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
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@GVodyanov please change the commit message to @hamza221 @SebastianKrupinski please test/review so we can get this in ASAP to give translators a chance to cover the new strings before the release. |
f5cb0c7 to
a56316b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clicking on "New address book" should open the input field that was previously top-level in a dialog titled "New address book", with options to "Cancel" or "Add" (primary).
- no cancel/add buttons
- On submit I would expect to revert back to the button ? (or at least on cancel)
- Sort by select is broken (can't pick value)
- Update avatars from social media should come after the sorting NcSelect according to the mockup
|
/backport to stable8.1 |
|
/backport to stable8.0 |
|
Given the translation string changes this shouldn't be backported too far. The goal is to have these changes released by server 32.0.2 release date. |
So down to 8.0 |
Right so sort by select being broken is something I decided to move to another PR #4804 I'll do the rest now |
|
I also converted the custom addressbook list into |
d1f7f99 to
fc0e0fc
Compare
|
Squash seems to have gone wrong. Only the commit message of the first commit should be taken |
|
Same note as for Calendar. "Sort by" now uses NcFormBoxSelectNative. |
fc0e0fc to
e4a1a5a
Compare
Signed-off-by: Grigory Vodyanov <[email protected]>
e4a1a5a to
19771fb
Compare
Can you please resolve conflicts and address this 👆🏼 |
|
|
Having said all of this, apparently @nfebe accidentally created a duplicate of this PR here #4865 After talking with him, we agreed that we will use his PR. It's newer and has all the Nextcloud vue components that weren't released at the time of creation of my PR. Please go ahead and review that one: #4865 |
Fix #4785
Here's how it's looking with the current state of the Nextcloud-vue components
