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

6 participants
@silverwind
Copy link
Contributor

silverwind commented Feb 2, 2019

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 added some commits Feb 2, 2019

@silverwind

This comment has been minimized.

Copy link
Contributor 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 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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor 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

This comment has been minimized.

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 and removed lgtm/need 2 labels Feb 2, 2019

@silverwind

This comment has been minimized.

Copy link
Contributor 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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

silverwind commented Feb 2, 2019

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

This comment has been minimized.

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 kind/ui label Feb 2, 2019

@zeripath zeripath added this to the 1.8.0 milestone Feb 2, 2019

@silverwind

This comment has been minimized.

Copy link
Contributor 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 and removed lgtm/need 1 labels Feb 2, 2019

@kolaente

This comment has been minimized.

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

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment