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.

Sign up for GitHub

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

Jump to bottom

Cleaner UI for explore page (#1253) #1255

Merged
merged 11 commits into from
Mar 15, 2017
Merged

Cleaner UI for explore page (#1253) #1255

merged 11 commits into from
Mar 15, 2017

Conversation

pgaskin
Copy link
Contributor

@pgaskin pgaskin commented Mar 14, 2017

Old New
https://cloud.githubusercontent.com/assets/13909717/23919233/73a7e6b8-08cc-11e7-9b49-c8a01f5a527a.png https://cloud.githubusercontent.com/assets/13909717/23920368/be1e7c08-08d0-11e7-8168-0d14a6fa6477.png

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 14, 2017 •
edited
Loading

#1253

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 14, 2017
@pgaskin pgaskin mentioned this pull request Mar 14, 2017
Closed
@pgaskin pgaskin changed the title Fix #1523 Cleaner UI for explore page (#1253) Mar 14, 2017
@strk
Copy link
Member

strk commented Mar 14, 2017

Trusted LGTM (trusting the screenshots :)

@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 Mar 14, 2017
@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 14, 2017

@strk Don't worry, I have applied and tested this on my server.

@bkcsoft
Copy link
Member

bkcsoft commented Mar 14, 2017

For Large screens (that's col-12-l in bootstrap FWIW) I'd love to have the search-bar and tab-bar be the same one. But that's just my preference 🙂 Otherwise 👍 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 Mar 14, 2017
@lunny lunny added this to the 1.2.0 milestone Mar 15, 2017
@lunny lunny added type/refactoring Existing code has been cleaned up. There should be no new functionality. topic/ui Change the appearance of the Gitea UI labels Mar 15, 2017
tboerger
tboerger reviewed Mar 15, 2017
View reviewed changes
public/less/_explore.less Outdated
@@ -3,6 +3,13 @@
padding-bottom: @footer-margin * 2;

.navbar {
justify-content: center !important;
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use important, just be more specific and you can avoid that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 15, 2017

Finished

@KangoV
Copy link

KangoV commented Mar 15, 2017

Would this look better if the tabs looked the same as tabs on other pages and left aligned? I'm all for a consistent look.

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 15, 2017

@KangoV I have tried left aligned and it does not look good.

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 15, 2017

Can this be merged now?

@bkcsoft bkcsoft merged commit 03d7998 into go-gitea:master Mar 15, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Mar 15, 2017

@geek1011 Mind going through the other pages that have this side-menu now? 😉

@pgaskin pgaskin deleted the patch-7 branch March 15, 2017 19:02
@tboerger
Copy link
Member

@KangoV maybe we should build it like that entirely ;)

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 15, 2017

@bkcsoft Do you have a list? 😉

@pgaskin
Copy link
Contributor Author

pgaskin commented Mar 15, 2017

@bkcsoft It looks pretty good with the admin, the repo, and the user settings, but the problem is the tabs do not scroll horizontally, but they work if I add overflow-x:auto and justify-content:left. Should I start now?

@pgaskin pgaskin mentioned this pull request Mar 15, 2017
Closed
@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.
Reviewers

@tboerger tboerger tboerger left review comments

@appleboy appleboy appleboy approved these changes

Assignees
No one assigned
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 type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
8 participants
@pgaskin @strk @bkcsoft @KangoV @tboerger @appleboy @lunny @geek1011