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

Responsive view #2750

Merged
merged 17 commits into from
Dec 31, 2017
Merged

Responsive view #2750

merged 17 commits into from
Dec 31, 2017

Conversation

thehowl
Copy link
Contributor

@thehowl thehowl commented Oct 20, 2017

Other pages will come in future PRs. This PR closes #35.

  • First of all, the meta viewport has been set to one, the basic for any responsive website.
  • Navbar has been simplified and refactored. It has some slightly different behaviour and look, but users without OCD should not see a big difference.
  • Main reason for the change is to have a better system where it was easier to plug in a stackable navbar, alongside a hamburger button for expanding or collapsing the navbar.
  • Dashboard, issues, and explore have been curated to make sure they look good on mobile devices.

Screenshots

New navbar

screenshot-2017-10-20 gitea

Example of collapsed navbar (and dashboard)

screen shot 2017-10-20 at 17 16 20

Collapsing/expanding demo

Issues page (I ask you to disregard the issue titles I use for testing 😉)

screen shot 2017-10-20 at 17 21 12-fullpage

Explore

screen shot 2017-10-20 at 17 21 48-fullpage

@thehowl thehowl mentioned this pull request Oct 20, 2017
@codecov-io
Copy link

codecov-io commented Oct 20, 2017

Codecov Report

Merging #2750 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2750      +/-   ##
==========================================
- Coverage   34.89%   34.73%   -0.16%     
==========================================
  Files         277      277              
  Lines       40177    40177              
==========================================
- Hits        14019    13955      -64     
- Misses      24099    24178      +79     
+ Partials     2059     2044      -15
Impacted Files Coverage Δ
modules/lfs/content_store.go 7.81% <0%> (-35.94%) ⬇️
modules/lfs/server.go 20.68% <0%> (-14.33%) ⬇️
models/lfs.go 26.08% <0%> (-2.18%) ⬇️
models/repo_list.go 67.18% <0%> (+1.56%) ⬆️
models/repo_indexer.go 53.88% <0%> (+1.94%) ⬆️
modules/indexer/repo.go 67.82% <0%> (+6.95%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f8c65c...207337e. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 20, 2017
@thehowl
Copy link
Contributor Author

thehowl commented Oct 20, 2017

Need some help here: at the moment forms are organised to be like this:

sharenixtemp tatddejzecgl6ethvxt

This does not really cope well with mobile devices, because this is what you get:

screen shot 2017-10-20 at 21 07 18-fullpage

Would it be OK to replace these custom forms with Semantic UI's stock forms, having the label of the field sitting on top of the field instead of on the side?

@hellasteph
Copy link

I use 2FA when logging into Gitea, any plans to support that in the initial responsive version?

@thehowl
Copy link
Contributor Author

thehowl commented Oct 20, 2017

That should be included in the task of getting forms to look good (which is what I need to do for the login anyway). In any case, 100% of Gitea's functionality after this PR will be on mobile, too: the only difference is that some pages will look weird for the moment (and to not make this PR even bigger)

@hellasteph
Copy link

I don't mind QA-ing if that will help you along. I greatly appreciate the work done so far!

@lunny lunny added this to the 1.x.x milestone Oct 21, 2017
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Oct 21, 2017
@lafriks
Copy link
Member

lafriks commented Oct 21, 2017

In mobile version input buttons must be full width and labels aligned to left

@pgaskin
Copy link
Contributor

pgaskin commented Oct 21, 2017

I will also test the changes too.

@pgaskin
Copy link
Contributor

pgaskin commented Oct 21, 2017

Great job so far.

Some things to work on:

  • The create menu is hard to use on mobile.
  • The initialize repository checkbox in the /repo/create is positioned far right off the page
  • The user/orgs/repo explore page's nav does not need to stack, it fits fine without, and it looks ugly when stacked
  • The file list in the repo is a bit too thick (maybe remove the last commit name text)

@thehowl thehowl changed the title [WIP] Responsive view Responsive view Oct 21, 2017
@thehowl
Copy link
Contributor Author

thehowl commented Oct 21, 2017

Hey there! Thanks for your feedback.

  • What do you mean by hard to use?
  • Fixed
  • Fixed, done also for the login page.
  • I'm not sure about this last one - unfortunately there is no pretty way to "hide" a column on tables only for mobile devices on semantic UI (due to its use of !importants). Perhaps a future PR could try to do that.

Also, I've pretty much implemented everything I previously said I would do in the first post and went ahead and removed the [WIP] tag. Reviews and further feedback welcome!

@lunny
Copy link
Member

lunny commented Oct 22, 2017

@thehowl CI failed.

@pgaskin
Copy link
Contributor

pgaskin commented Oct 22, 2017

By hard to use, I mean it sometimes disappears too soon (but I don't think you can do something about this issue).

@thehowl
Copy link
Contributor Author

thehowl commented Oct 24, 2017

Seems like last time the pgsql test randomly failed. Merged master and it works now ¯\_(ツ)_/¯ @lunny ready for review.

@lunny
Copy link
Member

lunny commented Oct 26, 2017

Please rebase

@thehowl
Copy link
Contributor Author

thehowl commented Oct 26, 2017

done

@jonasfranz
Copy link
Member

There are some conflicts with master.

@thehowl
Copy link
Contributor Author

thehowl commented Nov 4, 2017

Ready for review... again

@lunny
Copy link
Member

lunny commented Nov 5, 2017

image
When covert to an organization role on dashboard, UI broke.

@lunny
Copy link
Member

lunny commented Nov 5, 2017

image
explore repository UI should be improved.

@thehowl
Copy link
Contributor Author

thehowl commented Nov 11, 2017

@lunny addressed your issues and rebased ✅ sorry for taking long

@lunny
Copy link
Member

lunny commented Nov 13, 2017

@thehowl Good job!!! Maybe this PR could be merged before v1.4 released. But some issues below:

image

@thehowl
Copy link
Contributor Author

thehowl commented Nov 16, 2017

@lunny done & rebased

@lunny
Copy link
Member

lunny commented Nov 21, 2017

Repository home needs some improvement, otherwise LGTM
image

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 21, 2017
@lunny lunny removed this from the 1.x.x milestone Nov 21, 2017
@lunny lunny added this to the 1.4.0 milestone Nov 21, 2017
@lunny
Copy link
Member

lunny commented Dec 4, 2017

@thehowl need rebase

@thehowl
Copy link
Contributor Author

thehowl commented Dec 29, 2017

Sorry for the delay. Can we still get this in by 1.4.0 (in which case I'll proceed to the rebasing now) or since we're in the feature freeze already will this have to wait til 1.5?

@lunny
Copy link
Member

lunny commented Dec 30, 2017

@thehowl I would like this is in v1.4. Feature freeze means new feature coming PR will be in v1.5, not mean the PR which has been marked as v1.4.

The class was necessary, because this way the
dropdown doesn't assume the contents of the
selected item.
minor improvements

- make login/sign up in navbar stackable
- make navbar in explore and sign in not stackable

Change selected class in TestPullCompare

Fix typo that happened when rebasing

fix dashboard on org view

improve profile UI

Use clearing on file diff to fix broken UI caused by floating elements

remove unresolved merge conflict, and | Sanitize

Fix repo home not loading
@thehowl
Copy link
Contributor Author

thehowl commented Dec 30, 2017

And the tests are green ☑️

If anyone reviews the code, make sure to append ?w=1 to the URL of the diff so you can ignore the whitespace changes

@lafriks
Copy link
Member

lafriks commented Dec 30, 2017

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 30, 2017
@lunny lunny merged commit 3d3faa2 into go-gitea:master Dec 31, 2017
@daviian daviian mentioned this pull request Apr 26, 2018
7 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Responsive UI
9 participants