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.

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

Various css fixes #6669

Merged
merged 10 commits into from
Sep 28, 2017
Merged

Various css fixes #6669

merged 10 commits into from
Sep 28, 2017

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 27, 2017

Use filter instead of box-shadow Better contrast (includes the top arrow)
kazam_screenshot_00001 kazam_screenshot_00000
Implement progress theming (html5) Also revert text on quota bar and use primary
kazam_screenshot_00003 30912986-d9f0c54a-a38e-11e7-9274-6322d6eb89e5
kazam_screenshot_00005 30912983-d9d9d632-a38e-11e7-8ba6-e90f2cc18b5c
Rebuilt quota on user list Fixed some invalid colors too & fix #6654
kazam_screenshot_00008 kazam_screenshot_00007
kazam_screenshot_00009 kazam_screenshot_00006

@skjnldsv skjnldsv added 2. developing Work in progress design Design, UI, UX, etc. labels Sep 27, 2017
@skjnldsv skjnldsv added this to the Nextcloud 13 milestone Sep 27, 2017
@skjnldsv skjnldsv self-assigned this Sep 27, 2017
@codecov-io
Copy link

codecov-io commented Sep 27, 2017

Codecov Report

Merging #6669 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #6669      +/-   ##
============================================
- Coverage     53.03%   53.03%   -0.01%     
  Complexity    22583    22583              
============================================
  Files          1417     1417              
  Lines         87866    87873       +7     
  Branches       1341     1341              
============================================
+ Hits          46600    46601       +1     
- Misses        41266    41272       +6
Impacted Files Coverage Δ Complexity Δ
apps/files/templates/appnavigation.php 0% <ø> (ø) 0 <0> (ø) ⬇️
core/js/apps.js 50% <0%> (-4.55%) 0 <0> (ø)
settings/templates/users/part.userlist.php 0% <0%> (ø) 0 <0> (ø) ⬇️
...ings/templates/settings/personal/personal.info.php 0% <0%> (ø) 0 <0> (ø) ⬇️
apps/files_trashbin/lib/Trashbin.php 72.53% <0%> (+0.24%) 136% <0%> (ø) ⬇️

@MorrisJobke
Copy link
Member

Also revert text on quota bar and use primary

The default background of the quota bar should be white instead of grey (like in the files sidebar).

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 27, 2017

@MorrisJobke This has been requested by @jancborchardt :)

The default background of the quota bar should be white instead of grey (like in the files sidebar).

It's grey in the sidebar! :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 27, 2017

Quota update pushed!
See first post! :)

@skjnldsv
Copy link
Member Author

Okay, let's review @nextcloud/designers
I don't want to make too bigs pr with fixes. I will create new ones if I need!

CC @jancborchardt @MorrisJobke

@jancborchardt
Copy link
Member

Wow, looks great, all of it! 👍

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

I like ti a lot 👍

@MorrisJobke
Copy link
Member

Integration tests fail:

Scenario: log in with invalid user once fixed by admin              # /drone/src/github.com/nextcloud/server/tests/acceptance/features/login.feature:32
    Given I act as John                                               # ActorContext::iActAs()
    And I can not log in with user unknownUser and password 123456acb # LoginPageContext::iCanNotLogInWithUserAndPassword()
    When I act as Jane                                                # ActorContext::iActAs()
    And I am logged in as the admin                                   # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                      # SettingsMenuContext::iOpenTheUserSettings()
    And I create user unknownUser with password 123456acb             # UsersSettingsContext::iCreateUserWithPassword()
    And I see that the list of users contains the user unknownUser    # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
      Row for user unknownUser in Users Settings could not be found after 100 seconds (NoSuchElementException)
    And I act as John                                                 # ActorContext::iActAs()
    And I log in with user unknownUser and password 123456acb         # LoginPageContext::iLogInWithUserAndPassword()
    Then I see that the current page is the Files app                 # FilesAppContext::iSeeThatTheCurrentPageIsTheFilesApp()

And the upload progress bar is now grey instead of blue.

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 27, 2017

  • fix the upload progress!
  • fix tests, need help on this. @danxuliu?

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 28, 2017
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@danxuliu
Copy link
Member

I have also noticed a problem when the menu appears over the quota button:
menuonquota

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 28, 2017

Thank you so much @danxuliu ! :)
Will fix asap!!

  • Fix popover to css guidelines!

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

jancborchardt commented Sep 28, 2017

  • @skjnldsv just one detail I noticed: It looks strange that the quota bar (filling) has a border-radius on the top right and bottom right too. It wasn’t there before and looks much better without it. It’s ok for the small progress bars where the tip of the bar is half-round, but on the big quota bar this is strange. :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 28, 2017

Simple idea too, what do you think @jancborchardt @MorrisJobke ?
capture d ecran_2017-09-28_10-58-26

We could also implement this in the file list when dropping in a folder. Displaying a progressbar on the bottom of the row? :)

Instead of this!
capture d ecran_2017-09-28_16-26-31

@MorrisJobke
Copy link
Member

Simple idea too, what do you think @jancborchardt @MorrisJobke ?

Mmmmh ... I don't like the shadow around the time and the black theme is confusing. I need to see it also in real life to properly judge if it feels right. 😉

@MorrisJobke
Copy link
Member

I'm also fine with getting this in now and fix the progress bar design afterwards, if it gets an overhaul anyways. (to not make this even bigger and bigger)

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 28, 2017

Mmmmh ... I don't like the shadow around the time and the black theme is confusing. I need to see it also in real life to properly judge if it feels right. 😉

Do not judge the colours, the shadow isn't right because I didn't edit it ^^'

@MorrisJobke Completely agree, I won't add the progressbar into this one anyway, was just an idea like this :)
Will push the color fix in a moment!

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 28, 2017
@skjnldsv
Copy link
Member Author

@MorrisJobke @jancborchardt @danxuliu please review! :)
cc @nextcloud/designers ❤️

@jancborchardt
Copy link
Member

jancborchardt commented Sep 28, 2017

@skjnldsv The loong bar on the top is a bit much. Let’s just keep it a short progress bar where you can quickly see the percentage. :) Of course the current bar should be redesigned to use the new thin bar we use for quota too.

Otherwise good! :)

@skjnldsv
Copy link
Member Author

@jancborchardt is it a review? 😆

@jancborchardt
Copy link
Member

Yes, my review from earlier still stands :)

removed border-radius on big quota from the main settings page

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>
@skjnldsv skjnldsv merged commit d58c0e6 into master Sep 28, 2017
@skjnldsv skjnldsv deleted the various-css-fixes branch September 28, 2017 15:48
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 28, 2017
@enoch85
Copy link
Member

enoch85 commented Sep 28, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The quota button in the user settings table is not aligned vertically
6 participants