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: Better support for long repo names #5932

Merged
merged 2 commits into from
Feb 2, 2019

Conversation

silverwind
Copy link
Member

Small template tweak to allow more of the repo name to show without wrapping.

Before

screenshot 2019-02-02 at 07 53 07

After

screenshot 2019-02-02 at 07 52 59

@silverwind
Copy link
Member Author

silverwind commented Feb 2, 2019

Second commit does a similar thing to the news feed:

Before

screenshot 2019-02-02 at 08 00 17

After

screenshot 2019-02-02 at 08 00 07

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 2, 2019
@silverwind silverwind changed the title UI: Make long repo names wrap less often UI: Better support for long repo names Feb 2, 2019
@codecov-io
Copy link

codecov-io commented Feb 2, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5932      +/-   ##
==========================================
- Coverage   37.97%   37.96%   -0.02%     
==========================================
  Files         329      329              
  Lines       48406    48406              
==========================================
- Hits        18383    18377       -6     
- Misses      27381    27388       +7     
+ Partials     2642     2641       -1
Impacted Files Coverage Δ
models/unit.go 0% <0%> (-14.29%) ⬇️
routers/repo/view.go 46.1% <0%> (-1.2%) ⬇️

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 f9d4bd5...b1e5c52. Read the comment docs.

@silverwind
Copy link
Member Author

silverwind commented Feb 2, 2019

A proper fix for these kind of issues is flexbox, but it does not seem to be possible via semantic-ui, it's way too limited on what one can do and it does not even have a flex: 1 equivalent from what I gather. Maybe I'll try something later, but this commit here is a improvement, so can be merged.

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2019

I think we're happy to migrate to vue as necessary

@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 Feb 2, 2019
@silverwind
Copy link
Member Author

silverwind commented Feb 2, 2019

It's not vue-specific, you still need CSS (framework or hand-coded), and I think semantic-ui is too fat, outdated, full of weird hacks and (as of late) unmaintained. Maybe one of these:

https://bulma.io/
http://flexboxgrid.com/ (lightweight approach for only the flexbox part)

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2019

I don't think any of the maintainers have a huge amount of time to do a massive UI refactor but I think we're definitely of the opinion that it's time to migrate off semantic UI. The problem is how to do that without breaking everything.

If you're able and have the skills I think it's reasonable to do. The only thing I'd say is that you're gonna need lots of screenshots to prove its working.

@silverwind
Copy link
Member Author

Sorry, I don't think I have the motivation and time to do a UI rewrite, but I guess we could gradually move off semantic-ui, e.g. first add lightweight flexbox support (which start with just a couple of helper classes), then start preferring them over semantic for layout purposes.

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2019

Stream of thought follows:

  • I think you'd probably have trouble getting a total rewrite through the review process unless you formed a maintainer cabal.

  • How do we proceed with pushing forward with updating the UI? Ok, I think code speaks better than discourse but I wouldn't want to go down a black hole of making major changes without first discussing them first. The trouble is catching the interest of enough maintainers to get through the review process.

  • In a perfect world what framework would you propose? Given we're not in that perfect world and have a large amount of technical debt do you think that flexboxgrid is well supported enough and easy to migrate slowly across to?

  • We also need an easy way to check the feel of the UI changes - I don't think we can rely on automated tests here or simple code reviews. Unfortunately that means most of these changes won't be able to be reviewed on phone unless you put lots of screenshots on. I guess we also need UI testing "stories", human scripts to follow testing the UI and finding the break points.

  • You've had ~29 PR requests merged are you sure you wouldn't like to become a maintainer? (You don't get automatic merging rights but can approve/reject PRs)

@zeripath zeripath added the topic/ui Change the appearance of the Gitea UI label Feb 2, 2019
@zeripath zeripath added this to the 1.8.0 milestone Feb 2, 2019
@silverwind
Copy link
Member Author

silverwind commented Feb 2, 2019

How do we proceed with pushing forward with updating the UI

A total rewrite is out of question unless someone is willing to invest hundrets of hours, which I'm not. I suggest gradually introducing flexbox where it makes sense, small pieces that can be easily reviewed.

In a perfect world what framework would you propose

There's no perfect framework. Right now, flexbox layout is popular with css-grid also around the corner. I generally prefer do to things without frameworks because it offers the most control. Frameworks like semantic or bulma are too far abstracted from what is actually happening in CSS. A more modern approach is to use classes that map to exact css properties, and I think this is the way to go. Also, I'm not really a fan of CSS-in-JS because that leads to a lot of duplication.

Regarding UI layout testing, I have no experience unfortunately. I may consider becoming a maintainer later, but right now I prefer not to be.

@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 Feb 2, 2019
@kolaente
Copy link
Member

kolaente commented Feb 2, 2019

Can we move your disscussion about a move off semantic ui to a seperate issue to give more people a chance to discuss it? @zeripath @silverwind

@lafriks lafriks merged commit af22df8 into go-gitea:master Feb 2, 2019
@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.
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.

6 participants