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

UI polishing #71

Merged
merged 6 commits into from
Jul 24, 2018
Merged

UI polishing #71

merged 6 commits into from
Jul 24, 2018

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jul 11, 2018

From nextcloud/server#10094

  • Top image & logo missing
  • Next button needs more padding, to 44px min clickability
  • Icons on some entries missing
  • First slide should have a nice picture in content instead of text. (Like on Nextcloud flyer)
  • Fix mobile rendering
  • Adjust icons to new server classes after the files got unified in Svg color api server#9984

@juliusknorr
Copy link
Member Author

 Icons on some entries missing

@jancborchardt I cannot see any missing icon there, can you point me to what exactly is missing

 First slide should have a nice picture in content instead of text. (Like on Nextcloud flyer)

@jancborchardt Do you have that image at hand somewhere? Or maybe @jospoortvliet

And should this image replace the header area with the background image or the content, where we have the core values? Because then we would need to move the core values to a separate page.

@jancborchardt
Copy link
Member

jancborchardt commented Jul 17, 2018

@juliushaertl sent you the link to the background image via Nextcloud Talk. ;)

I can actually not reproduce the icons not loading, maybe another problem specific to Franks setup. But I found 2 new issues:

  • Icons on first slide are not correctly shown on mobile – might be solved by simply using the image
  • Scrolling is very slow cause it scrolls the whole page (see the whole page behind the modal being scrolled up)

screenshot from 2018-07-17 21-07-09

@jancborchardt
Copy link
Member

And the image should replace the icons / text boxes on the bottom. And if I remember correctly (correct me if I’m wrong @jospoortvliet) we decided that we do not insert another slide for it, because it’s just too much blabla that people will skip anyhow.

@juliusknorr
Copy link
Member Author

@jancborchardt The icons about the core values of Nextcloud were actually the main requirement for the firstrunwizard rework. See #57 so I wonder why those are supposed to be dropped again. Also just using the image instead of the text/icons as a first slide doesn't provide any value to the user: https://user-images.githubusercontent.com/3404133/43079564-beae9cc2-8e8d-11e8-959d-c7beb300f480.png

But I like the idea of using that image, maybe as the header image:
image

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@MorrisJobke
Copy link
Member

@juliushaertl Should we get this in like it is now and finish the polishing later?

@MorrisJobke MorrisJobke mentioned this pull request Jul 24, 2018
21 tasks
@juliusknorr
Copy link
Member Author

@MorrisJobke Good to go from my side.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
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 👍

@MorrisJobke MorrisJobke merged commit dce1fb3 into master Jul 24, 2018
@MorrisJobke MorrisJobke deleted the bugfix/noid/images branch July 24, 2018 14:40
<?php p($l->t('Connect your calendar'));?>
</a>
<a target="_blank" class="button" href="<?php p(link_to_docs('user-sync-contacts')) ?>">
<img class="appsmall appsmall-contacts svg" alt=""
src="<?php p(image_path('core', 'places/contacts-dark.svg')); ?>" />
src="<?php p(image_path('core', 'places/contacts.svg')); ?>" />
Copy link
Member

Choose a reason for hiding this comment

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

@juliushaertl I fixed the icons in here as well 😉

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

Successfully merging this pull request may close these issues.

3 participants