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.

Sign up for GitHub

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

Jump to bottom

Fit contacts list to guidelines #438

Merged
merged 5 commits into from
Jan 11, 2018
Merged

Fit contacts list to guidelines #438

merged 5 commits into from
Jan 11, 2018

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 17, 2017 •
edited
Loading

@skjnldsv skjnldsv added 2. developing Work in progress design Related to the design high High priority labels Nov 17, 2017
@skjnldsv skjnldsv self-assigned this Nov 17, 2017
@codecov
Copy link

codecov bot commented Nov 17, 2017 •
edited
Loading

Codecov Report

Merging #438 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #438      +/-   ##
=========================================
+ Coverage   15.78%   15.8%   +0.02%     
=========================================
  Files          61      61              
  Lines        1381    1379       -2     
=========================================
  Hits          218     218              
+ Misses       1163    1161       -2
Impacted Files Coverage Δ
js/components/contact/contact_controller.js 9.09% <ø> (+1.39%) ⬆️

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 e97a0a8...4d320f4. Read the comment docs.

@skjnldsv skjnldsv force-pushed the list-to-css-guidelines branch 2 times, most recently from 700dd84 to af0188e Compare November 18, 2017 10:09
@skjnldsv skjnldsv mentioned this pull request Nov 20, 2017
Merged
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 21, 2017
@irgendwie
Copy link
Member

Compatible with at least Nextcloud 12?

@skjnldsv
Copy link
Member Author

@irgendwie No, patch for 12 will come in next pr :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 6, 2017

Can we please get this in?

@irgendwie
Copy link
Member

2017-12-07-172354_1137x293_scrot

  • Scrolling in the contacts list completely broken for me
  • White space on the right next to the delete icon

Only for me?

irgendwie
irgendwie previously requested changes Dec 7, 2017
View reviewed changes
Copy link
Member

@irgendwie irgendwie left a comment

Choose a reason for hiding this comment

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

see last comment

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 7, 2017

@irgendwie nope, here as well, this is due to server, I have a patch coming asap. Don't know how I missed it....

@skjnldsv skjnldsv mentioned this pull request Dec 8, 2017
Merged
MorrisJobke
MorrisJobke approved these changes Dec 11, 2017
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke 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 - even in Edge and IE11 😉

@skjnldsv skjnldsv force-pushed the list-to-css-guidelines branch from c17b1cc to 3b6ccea Compare December 11, 2017 16:55
@skjnldsv
Copy link
Member Author

Rebased for merging

jancborchardt
jancborchardt requested changes Dec 11, 2017
View reviewed changes
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.

  • When reducing the width, shortly before it breaks to mobile view, there’s a horizontal scrollbar.
  • When the width is reduced and you click on a contact, the app-content-list briefly expands width while loading the new contact.

@skjnldsv skjnldsv mentioned this pull request Dec 11, 2017
Closed
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 11, 2017
@skjnldsv
Copy link
Member Author

When reducing the width, shortly before it breaks to mobile view, there’s a horizontal scrollbar.
When the width is reduced and you click on a contact, the app-content-list briefly expands width while loading the new contact.

Can't reproduce, still happening with latest commits from nextcloud/server#7195 ?

@jancborchardt
Copy link
Member

Two more strange issues:
In Firefox, for entries with only a name the avatar sometimes glitches to the top (sometimes only when zooming, sometimes jitters a bit when loading the app):
screenshot from 2017-12-12 14-29-28

In Chrome, the app-content-list is strangely narrow for contacts which have 2 columns (censored the contact names but it’s still very narrow, see the scrollbar/divider):
screenshot from 2017-12-12 14-32-02

@skjnldsv
Fit contacts list to guidelines
891a511 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Removed star
00300d1 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Bring back the hamburger
f62578a 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Fixed csslint
90946d2 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the list-to-css-guidelines branch from 7460f6a to 90946d2 Compare December 21, 2017 09:55
@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 21, 2017 •
edited
Loading

@jancborchardt this is related to the column system! :/
@irgendwie is this fixed now? With latest nc13b3?

@skjnldsv
Fixed import screen
4d320f4 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@jancborchardt
Copy link
Member

@skjnldsv yeah, thought so :/

@nextcloud/contacts what do you think - let's get this in for 13 and fix going forward?

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 9, 2018
@skjnldsv
Copy link
Member Author

skjnldsv commented Jan 9, 2018

@jancborchardt can you update your review? :)

jancborchardt
jancborchardt approved these changes Jan 9, 2018
View reviewed changes
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.

It's ok for now but let's also get the details fixed. :)

@skjnldsv skjnldsv merged commit b1fcfb1 into master Jan 11, 2018
@skjnldsv skjnldsv deleted the list-to-css-guidelines branch January 11, 2018 09:38
@skjnldsv skjnldsv mentioned this pull request Jan 14, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonatoni jonatoni jonatoni approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@irgendwie irgendwie irgendwie left review comments

@Henni Henni Awaiting requested review from Henni

@xh3n1 xh3n1 Awaiting requested review from xh3n1

Assignees

@skjnldsv skjnldsv

Labels
3. to review Waiting for reviews design Related to the design high High priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@skjnldsv @irgendwie @jancborchardt @MorrisJobke @jonatoni