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

ui - issue view - fix icon position #7354

Merged
merged 9 commits into from
Jul 6, 2019
Merged

ui - issue view - fix icon position #7354

merged 9 commits into from
Jul 6, 2019

Conversation

Cherrg
Copy link
Contributor

@Cherrg Cherrg commented Jul 4, 2019 •
edited
Loading

issue view - fix icon position

  • move style from template to css
  • add bullets to: key, lock, circle-slash, comment

Signed-off-by: Michael Gnehr [email protected]

Images

Old New Old dark New dark dark (with #7355)
grafik grafik grafik grafik grafik

other commits:

  • (2nd) add border to symbols without bullet
result (2nd) change color to display added border (2nd) update top alignment
grafik grafik grafik grafik

@Cherrg
issue view - fix icon position
f864230 
- move style from template to css
- add bullets to: key, circle-slash, comment

Signed-off-by: Michael Gnehr <[email protected]>
@Cherrg Cherrg changed the title issue view - fix icon position ui - issue view - fix icon position Jul 4, 2019
Cherrg added 2 commits July 4, 2019 17:01
@Cherrg
add border to symbols
7cfc573 
Signed-off-by: Michael Gnehr <[email protected]>
@Cherrg
fix circle slash position
f0237b3 
Signed-off-by: Michael Gnehr <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 4, 2019
@silverwind
Copy link
Member

There's some mixed tab/space indentation in those template files, some probably existed before this commit. Maybe you wanna fix them:

.git/rebase-apply/patch:133: space before tab in indent.
     	</div>
.git/rebase-apply/patch:137: space before tab in indent.
	    	<span class="octicon octicon-{{.Review.Type.Icon}} issue-symbol"></span>
.git/rebase-apply/patch:138: space before tab in indent.
	    	<a class="ui avatar image" href="{{.Poster.HomeLink}}">
.git/rebase-apply/patch:139: space before tab in indent.
	    		<img src="{{.Poster.RelAvatarLink}}">
.git/rebase-apply/patch:140: space before tab in indent.
	    	</a>
warning: 1 line applied after fixing whitespace errors.

@silverwind
Copy link
Member

Suggestion: Align icon vertically with avatar, if it's not too hard:

Screenshot 2019-07-04 at 17 47 06

silverwind
silverwind approved these changes Jul 4, 2019
View reviewed changes
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

LGTM with above suggestions.

@GiteaBot GiteaBot 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 Jul 4, 2019
@Cherrg
fix top margin
c6be4d0 
Signed-off-by: Michael Gnehr <[email protected]>
@Cherrg
Copy link
Contributor Author

Cherrg commented Jul 4, 2019 •
edited
Loading

There's some mixed tab/space indentation in those template files, some probably existed before this commit. Maybe you wanna fix them:

.git/rebase-apply/patch:133: space before tab in indent.
     	</div>
.git/rebase-apply/patch:137: space before tab in indent.
	    	<span class="octicon octicon-{{.Review.Type.Icon}} issue-symbol"></span>
.git/rebase-apply/patch:138: space before tab in indent.
	    	<a class="ui avatar image" href="{{.Poster.HomeLink}}">
.git/rebase-apply/patch:139: space before tab in indent.
	    		<img src="{{.Poster.RelAvatarLink}}">
.git/rebase-apply/patch:140: space before tab in indent.
	    	</a>
warning: 1 line applied after fixing whitespace errors.

which command show me this list?

Edit: changed mixed indention to tab indent

@Cherrg
Copy link
Contributor Author

Cherrg commented Jul 4, 2019

Suggestion: Align icon vertically with avatar, if it's not too hard:

Screenshot 2019-07-04 at 17 47 06

should be ok now

@silverwind
Copy link
Member

silverwind commented Jul 4, 2019 •
edited
Loading

which command show me this list?

You can see them during git apply on a clean branch:

$ curl -sL https://github.com/go-gitea/gitea/pull/7354.patch | git apply
<stdin>:142: space before tab in indent.
     	</div>
<stdin>:146: space before tab in indent.
	    	<span class="octicon octicon-{{.Review.Type.Icon}} issue-symbol"></span>
<stdin>:147: space before tab in indent.
	    	<a class="ui avatar image" href="{{.Poster.HomeLink}}">
<stdin>:148: space before tab in indent.
	    		<img src="{{.Poster.RelAvatarLink}}">
<stdin>:149: space before tab in indent.
	    	</a>
warning: squelched 20 whitespace errors
warning: 18 lines applied after fixing whitespace errors.

I don't think it's possible to show those warnings on the active branch.

@silverwind
Copy link
Member

Icon alignment looking good, thanks.

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Jul 5, 2019
@GiteaBot GiteaBot 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 Jul 5, 2019
@zeripath zeripath added this to the 1.9.0 milestone Jul 6, 2019
@techknowlogick techknowlogick merged commit 3621000 into go-gitea:master Jul 6, 2019
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@5b92bc1). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7354   +/-   ##
=========================================
  Coverage          ?   41.22%           
=========================================
  Files             ?      467           
  Lines             ?    63363           
  Branches          ?        0           
=========================================
  Hits              ?    26119           
  Misses            ?    33828           
  Partials          ?     3416

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 5b92bc1...4f8a364. Read the comment docs.

jeffliu27 pushed a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019
@Cherrg @jeffliu27
ui - issue view - fix icon position (go-gitea#7354)
a8e4de0 
* issue view - fix icon position

- move style from template to css
- add bullets to: key, circle-slash, comment

Signed-off-by: Michael Gnehr <[email protected]>

* add border to symbols

Signed-off-by: Michael Gnehr <[email protected]>

* fix circle slash position

Signed-off-by: Michael Gnehr <[email protected]>

* fix top margin

Signed-off-by: Michael Gnehr <[email protected]>

* changed mixed space/tab indent to tabindent only

Signed-off-by: Michael Gnehr <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@kolaente kolaente kolaente 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
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Cherrg @silverwind @codecov-io @kolaente @lunny @techknowlogick @zeripath @GiteaBot