Skip to content

Icon on apps settings#6627

Merged
nickvergessen merged 2 commits intomasterfrom
icon-on-apps-settings
Sep 25, 2017
Merged

Icon on apps settings#6627
nickvergessen merged 2 commits intomasterfrom
icon-on-apps-settings

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 23, 2017

Fix #3987

@skjnldsv skjnldsv added the 2. developing Work in progress label Sep 23, 2017
@skjnldsv skjnldsv self-assigned this Sep 23, 2017
@mention-bot
Copy link

mention-bot commented Sep 23, 2017

@skjnldsv, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen, @LukasReschke and @ChristophWurst to be potential reviewers.

@codecov
Copy link

codecov bot commented Sep 24, 2017 •
edited
Loading

Codecov Report

Merging #6627 into master will not change coverage.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master    #6627   +/-   ##
=========================================
  Coverage     53.06%   53.06%           
  Complexity    22553    22553           
=========================================
  Files          1414     1414           
  Lines         87745    87745           
  Branches       1340     1340           
=========================================
  Hits          46565    46565           
  Misses        41180    41180
Impacted Files Coverage Δ Complexity Δ
settings/templates/apps.php 0% <0%> (ø) 0 <0> (ø) ⬇️
settings/js/apps.js 24.29% <0%> (ø) 0 <0> (ø) ⬇️

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

skjnldsv commented Sep 24, 2017

I tried to use most of stuff we already use on nextcloud to avoid confusion.
I created/edited some others. Please suggest a modification/new one if you don't like those.
There are some icons (like the key) which I don't like on my pr, I would be glad to see you suggest something else @nextcloud/designers :)

Thanks!
capture d ecran_2017-09-24_08-47-04

@skjnldsv
Icons & default classes
308e1c0 
Changed categories order same as app store

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the icon-on-apps-settings branch from 3fbd957 to 308e1c0 Compare September 24, 2017 06:59
ChristophWurst
ChristophWurst previously requested changes Sep 25, 2017
View reviewed changes
settings/js/apps.js Outdated
{displayName: t('settings', 'Enabled apps'), ident: 'enabled', id: '0'},
{displayName: t('settings', 'Disabled apps'), ident: 'disabled', id: '1'},
{displayName: t('settings', 'Your apps'), ident: 'installed', id: '2'}
var categories = [,
Copy link
Member

@ChristophWurst ChristophWurst Sep 25, 2017

Choose a reason for hiding this comment

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

remove the , please

settings/js/apps.js Outdated
{displayName: t('settings', 'Your apps'), ident: 'installed', id: '2'}
var categories = [,
{displayName: t('settings', 'Your apps'), ident: 'installed', id: '0'},
{displayName: t('settings', 'Enabled apps'), ident: 'enabled', id: '1',},
Copy link
Member

@ChristophWurst ChristophWurst Sep 25, 2017

Choose a reason for hiding this comment

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

remove the , please

Copy link
Member Author

@skjnldsv skjnldsv Sep 25, 2017

Choose a reason for hiding this comment

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

How did I missed that... 🙈
Thanks!!

@skjnldsv
Fix typo
9ff6dab 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv dismissed ChristophWurst’s stale review September 25, 2017 11:07

Fixed! Thanks!!! :)

MorrisJobke
MorrisJobke approved these changes Sep 25, 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.

Awesome 🚀 I like it

bildschirmfoto 2017-09-25 um 15 50 06

@MorrisJobke MorrisJobke added this to the Nextcloud 13 milestone Sep 25, 2017
@MorrisJobke MorrisJobke mentioned this pull request Sep 25, 2017
Closed
@nickvergessen nickvergessen merged commit f2b7205 into master Sep 25, 2017
@nickvergessen nickvergessen deleted the icon-on-apps-settings branch September 25, 2017 14:29
@jancborchardt
Copy link
Member

jancborchardt commented Sep 25, 2017

Awesome stuff @skjnldsv I like it a lot! :)

The key is good too! Maybe only mirror it, so that the key ring is on the top right instead – that’s how keys are most often visualized.

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 25, 2017

@jancborchardt damn, few seconds too late! 😕
Feel free to provide a pr for that if you really want! 😉

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

Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

@skjnldsv skjnldsv

Labels

3. to review Waiting for reviews design Design, UI, UX, etc. feature: apps management

Projects

None yet

Milestone

Nextcloud 13

Development

Successfully merging this pull request may close these issues.

6 participants

@skjnldsv @mention-bot @jancborchardt @nickvergessen @MorrisJobke @ChristophWurst