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

Simplify settings pages with item list #1389

Merged
merged 4 commits into from
Mar 30, 2017

Conversation

sapk
Copy link
Member

@sapk sapk commented Mar 26, 2017

Gitea SSH key seeting page display a column with a point indicating if the key is used recently.
It's more simple to display recent use of key like github and color the key icon and date of recent use.

Before :
before-deploy-keys
before-apps
before-keys
before-email

After :
after-deploy-keys
after-apps
after-keys
after-email

Still need to be apply in repository key list.

@sapk sapk changed the title Remove column point of ssh keys setting page [WIP] Remove column point of ssh keys setting page Mar 27, 2017
@lunny lunny added this to the 1.2.0 milestone Mar 27, 2017
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Mar 27, 2017
@sapk sapk changed the title [WIP] Remove column point of ssh keys setting page Simplify settings pages with item list Mar 28, 2017
@sapk
Copy link
Member Author

sapk commented Mar 28, 2017

I have gone a little further in little cleaning. In some case there we're item with grid with only one column and other with a "useless" column (the point) for indicating activity when it could be display in icon.

Before :
before-deploy-keys
before-apps
before-keys
before-email

After :
after-deploy-keys
after-apps
after-email

Note : I don't know why lessc add !important and simplify color #bbb it also does it on master branch ...

@strk
Copy link
Member

strk commented Mar 29, 2017

Why not keeping the octicon-key icon in deploy-keys as in your first screenshot ?

About !important CSS I'm afraid I removed that with a9de85d by using lessc 1.6.3 (LESS Compiler) [JavaScript] (lessc --version). Similar to the gofmt issue, this may be one of those cases in which different tooling version result in different behavior, which would require us to restrict the allowed version to run generative commands (see #1366)

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 29, 2017
@strk
Copy link
Member

strk commented Mar 29, 2017

Please also check if we need to update the templates version (I'm pretty sure Gogs had the concept of a "template version" to make sure templates and code reading them would be consistent, not sure if that concept is still working in Gitea)

@sapk
Copy link
Member Author

sapk commented Mar 29, 2017

@strk I have use a icon from semantic because it need the class .icon to be well display in .item but the class .icon override .octicon-* classes so I will need add some "hack" to fix it. If we want to keep octicon icon.

@sapk
Copy link
Member Author

sapk commented Mar 29, 2017

Looking at it should be only add
.ui.list > .item > i.octicon (or mega-octicon){
display: table-cell;
margin: 0;
padding-top: .07142857em;
padding-right: .28571429em;
vertical-align: top;
-webkit-transition: color .1s ease;
transition: color .1s ease;
}
based from .ui.list > .item > i.icon

@sapk
Copy link
Member Author

sapk commented Mar 29, 2017

I replace key icon with original octicon key
image

@sapk sapk force-pushed the remove-point-ssh-keys branch 2 times, most recently from f5d1420 to 90aba36 Compare March 29, 2017 17:48
@strk
Copy link
Member

strk commented Mar 29, 2017

LGTM

@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 29, 2017
@lunny
Copy link
Member

lunny commented Mar 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 Mar 30, 2017
@lunny lunny merged commit 912b340 into go-gitea:master Mar 30, 2017
@Morlinest Morlinest mentioned this pull request Mar 30, 2017
7 tasks
@Morlinest
Copy link
Member

Morlinest commented Apr 3, 2017

@sapk Why you delete ".key" class from less file? I am working (testing) on patch to fix multiple broken pages after your commit and this one missing word is problem. You changed styling in whole "settings" scope. Can i revert this little change?

@sapk
Copy link
Member Author

sapk commented Apr 3, 2017

This PR simplify the html and remove some specific css "hack" with a more generic and global css for all items lists.

@sapk
Copy link
Member Author

sapk commented Apr 3, 2017

In fact the .key is not completely removed but only the .key.meta specific code is kept for color and better spacing between line.

@sapk
Copy link
Member Author

sapk commented Apr 3, 2017

Looking at your issue, I will have a look if it doesn't have impacted others pages

@Morlinest
Copy link
Member

Morlinest commented Apr 3, 2017

@sapk It has impact also on dropdown options (repo ->collaborators -> user rights). If i revert ".key" in less file it looks OK and your change should still work fine (tested booth shh key settings).

@sapk
Copy link
Member Author

sapk commented Apr 5, 2017

Make a PR because I don't see exactly what you mean.
For dropdown impact just restrict parent with > is enough without further impact.

I will force me to take time to propose a quick solution or look at your PR soon.
I think you catch that more part of template could be simplify and follow more semantic ui design overall (so +1 for the good catch). That could be done in a other PR after a simple fix.

@sapk sapk mentioned this pull request Apr 6, 2017
@sapk sapk deleted the remove-point-ssh-keys branch April 28, 2017 10:04
@cez81 cez81 mentioned this pull request Jun 17, 2017
@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.

5 participants