Skip to content

Responsive app menu - #4439

Merged
LukasReschke merged 3 commits into
masterfrom
appmenu-responsive
Apr 25, 2017
Merged

Responsive app menu#4439
LukasReschke merged 3 commits into
masterfrom
appmenu-responsive

Conversation

@juliusknorr

@juliusknorr juliusknorr commented Apr 22, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

ezgif-2-4a1f35fd11

As discussed at the hackweek with @jancborchardt

ToDo:

  • Fix loading icon
  • Enabling/disabling apps doesn't show up nicely

@juliusknorr juliusknorr added 2. developing Work in progress design Design, UI, UX, etc. enhancement labels Apr 22, 2017
@juliusknorr juliusknorr added this to the Nextcloud 12.0 milestone Apr 22, 2017
@icewind1991

icewind1991 commented Apr 24, 2017

Copy link
Copy Markdown
Member

This manages to break the file list for me, it doesn't show any files in the list

file list

jancborchardt
jancborchardt previously requested changes Apr 24, 2017
View reviewed changes

@jancborchardt jancborchardt left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can confirm @icewind1991's issue. It also results in a double scrollbar on the right in Firefox, and there are too few items shown by default (should be 8 icons).

(Screenshot is after scrolling, so the files are visible – shows double scrollbar and too few icons.)
capture du 2017-04-24 18-47-48

cc @juliushaertl

@juliusknorr
Responsive app menu
7548825 
Signed-off-by: Julius Härtl <[email protected]>
@juliusknorr
Fix adding/removing apps
673b58e 
Signed-off-by: Julius Härtl <[email protected]>
@juliusknorr
Fix loading spinner for new app menu
27b19aa 
Signed-off-by: Julius Härtl <[email protected]>
@codecov

codecov Bot commented Apr 25, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #4439 into master will increase coverage by 0.19%.
The diff coverage is 27.69%.

@@             Coverage Diff              @@
##             master    #4439      +/-   ##
============================================
+ Coverage      53.9%   54.09%   +0.19%     
+ Complexity    21701    21695       -6     
============================================
  Files          1272     1331      +59     
  Lines         75815    83079    +7264     
  Branches          0     1315    +1315     
============================================
+ Hits          40870    44944    +4074     
- Misses        34945    38135    +3190
Impacted Files Coverage Δ Complexity Δ
lib/private/legacy/app.php 53.4% <ø> (+3.4%) 222 <0> (-6) ⬇️
lib/private/TemplateLayout.php 0% <ø> (ø) 35 <0> (ø) ⬇️
settings/js/apps.js 27.47% <0%> (ø) 0 <0> (?)
core/templates/layout.user.php 0% <0%> (ø) 0 <0> (ø) ⬇️
core/js/js.js 61.69% <46.15%> (ø) 0 <0> (?)
apps/comments/lib/EventHandler.php 79.16% <0%> (-8.34%) 7% <0%> (ø)
lib/private/User/Manager.php 83.81% <0%> (-0.58%) 68% <0%> (ø)
apps/files_sharing/js/public.js 47.91% <0%> (ø) 0% <0%> (?)
core/js/oc-requesttoken.js 100% <0%> (ø) 0% <0%> (?)
core/js/files/client.js 87.62% <0%> (ø) 0% <0%> (?)
... and 58 more

@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 25, 2017
@juliusknorr

juliusknorr commented Apr 25, 2017

Copy link
Copy Markdown
Member Author

Ready for review @nextcloud/designers

@juliusknorr

juliusknorr commented Apr 25, 2017

Copy link
Copy Markdown
Member Author

@icewind1991 Thanks for having a look. Should be fixed now, there was some leftover css rule from development. 🙈

MorrisJobke
MorrisJobke approved these changes Apr 25, 2017
View reviewed changes

@MorrisJobke MorrisJobke left a comment

Copy link
Copy Markdown
Member

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 ;)) 👍

skjnldsv
skjnldsv reviewed Apr 25, 2017
View reviewed changes
Comment thread core/css/header.scss
height: 16px;
}
.app-icon {
-ms-filter: 'progid:DXImageTransform.Microsoft.Alpha(Opacity=0)';

@skjnldsv skjnldsv Apr 25, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need those ugly/strange opacity filters for ie?

skjnldsv
skjnldsv approved these changes Apr 25, 2017
View reviewed changes

@skjnldsv skjnldsv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Incredible work! :)

jancborchardt
jancborchardt approved these changes Apr 25, 2017
View reviewed changes

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works great now! 👍

@jancborchardt

jancborchardt commented Apr 25, 2017

Copy link
Copy Markdown
Member

Awesome work @juliushaertl! This is so cool :)

@MorrisJobke @LukasReschke @rullzer what do we do about the codecov patch diff?

@LukasReschke

LukasReschke commented Apr 25, 2017

Copy link
Copy Markdown
Member

@MorrisJobke @LukasReschke @rullzer what do we do about the codecov patch diff?

Ideally in a world where everything is awesome: Add tests for this 😉

/me hides 😉

@LukasReschke
LukasReschke merged commit db94b5d into master Apr 25, 2017
@LukasReschke
LukasReschke deleted the appmenu-responsive branch April 25, 2017 17:42
@LukasReschke

LukasReschke commented Apr 25, 2017

Copy link
Copy Markdown
Member

… as that isn't the case => merged 😉

@ChristophWurst ChristophWurst mentioned this pull request Apr 25, 2017
Merged
@eppfel eppfel mentioned this pull request Apr 26, 2017
Closed
@jancborchardt jancborchardt mentioned this pull request May 15, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes
@jancborchardt jancborchardt jancborchardt approved these changes
@skjnldsv skjnldsv skjnldsv approved these changes
@rullzer rullzer Awaiting requested review from rullzer
@nickvergessen nickvergessen Awaiting requested review from nickvergessen
@LukasReschke LukasReschke Awaiting requested review from LukasReschke

Assignees

No one assigned

Labels

3. to review Waiting for reviews design Design, UI, UX, etc. enhancement

Projects

None yet

Milestone

Nextcloud 12.0

Development

Successfully merging this pull request may close these issues.

6 participants

@juliusknorr @icewind1991 @jancborchardt @LukasReschke @MorrisJobke @skjnldsv