Skip to content

Apps scss optimisation - #3024

Merged
skjnldsv merged 8 commits into
masterfrom
apps-scss-optimisation
Jan 13, 2017
Merged

Apps scss optimisation#3024
skjnldsv merged 8 commits into
masterfrom
apps-scss-optimisation

Conversation

@skjnldsv

@skjnldsvskjnldsv commented Jan 11, 2017

Copy link
Copy Markdown
Member

Optimisation and simplification of the apps.scss file

  • Popover unification (see [styleguide] Popover menu #2798)
    • Unification
    • Added left/center positionning
    • Comments
    • Cleanup
  • Fusion of similar properties
  • Comments
  • Cleanup

@nextcloud/designers
Developer manual will be updated shortly

@skjnldsvskjnldsv added 2. developing Work in progress design Design, UI, UX, etc. enhancement feature: scss labels Jan 11, 2017
@skjnldsvskjnldsv self-assigned this Jan 11, 2017
@skjnldsvskjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 11, 2017
fix#2798
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

@ChristophWurstChristophWurst 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.

breaks the account popover in Mail (the one in the sidebar left)

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 11, 2017
@skjnldsv

skjnldsv commented Jan 11, 2017

Copy link
Copy Markdown
MemberAuthor

@ChristophWurst you mean the alignment on the right?

Also, the menu doesn't fit our css guidelines. You should have an horizontal menu with the icons.
using ul >li and repeating the ul isn't a good way to do so. :)

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv

Copy link
Copy Markdown
MemberAuthor

@ChristophWurst Okay, fixed.

@skjnldsvskjnldsv removed the 2. developing Work in progress label Jan 11, 2017
@ChristophWurst

Copy link
Copy Markdown
Member

Also, the menu doesn't fit our css guidelines. You should have an horizontal menu with the icons.
using ul >li and repeating the ul isn't a good way to do so. :)

fix it 😉

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv

Copy link
Copy Markdown
MemberAuthor

@ChristophWurst Done 😆

@codecov-io

codecov-io commented Jan 11, 2017

Copy link
Copy Markdown

Current coverage is 54.05% (diff: 100%)

Merging #3024 into master will increase coverage by 0.13%

@@ master #3024 diff @@
==========================================
Files 1302 1302 Lines 80061 80403 +342 Methods 7902 7977 +75 Messages 0 0 Branches 1245 1245 ==========================================
+ Hits 43171 43461 +290 - Misses 36890 36942 +52 
Partials 0 0 

Powered by Codecov. Last update bc26f78...ce9c833

@MorrisJobkeMorrisJobke 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 looks good 👍

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 13, 2017
@skjnldsv
skjnldsv merged commit 8d8aff1 into masterJan 13, 2017
@skjnldsv
skjnldsv deleted the apps-scss-optimisation branch January 13, 2017 15:31
@skjnldsvskjnldsv mentioned this pull request Feb 15, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishdesignDesign, UI, UX, etc.feature: scss

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@skjnldsv@ChristophWurst@codecov-io@MorrisJobke