-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add scroolbars to wide graphs. #608
Conversation
less file? |
What do you mean? I just made som changes in the existing template, css and js for the graph. |
Maybe you have to modify less file since you have modified css file. |
There is no less file. Its just a standard css file for this feature.
|
#rev-container {width:80%} | ||
#rev-list {margin:0;padding:0 5px 0 0;width:80%} | ||
#rev-container {width:100%} | ||
#rev-list {margin:0;padding:0 5px 0 0;width:100%} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't padding: 0 5px 0 5px; min-width: 95%
be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry. did not notice the 'min' part. min-width looks better :)
@@ -1,7 +1,7 @@ | |||
{{template "base/head" .}} | |||
<div class="repository commits"> | |||
{{template "repo/header" .}} | |||
<div class="ui container"> | |||
<div class="ui"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for something else (don't remember what ) I did a container.full
which sets the width to "100%"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats probably better :)
for further improvements, it might be possible to make the graph more compact, but then We would have to change the lib graph.js, this should probably be done upstream for the lib. It accepts one parameter for "unitSize", We now use default value, which is 20. But If we could set unitSize-x to 10 and unitSize-y to 20, the graph could be more compact and still have the same height. (the y value have to match the font-size used for the text.) but, I am not sure how many parallel branches there is useful to display. Maybe there should be a limit? |
@kjellkvinge Maybe you could add a horizontal scrollbar. Otherwise we'll keep running into the same problem. |
LGTM |
@kjellkvinge Nitpicking: could reset the width to 80%? I personally think the width of 100% looks kinda ugly 😅 |
@Bwko yes, this is difficult :) you mean somethink like this: -#git-graph-container {overflow-x:auto; width:100%}
+#git-graph-container {overflow-x:auto; width:80%; margin-left:10%} This will bring more air on the sides, but it looks miss placed when you have a repo with only one, or a few branches in parallell. it also depends on the screen size. maybe we should keep the "ui container" unless there is more than 5 or 10 paralell branches?? any thoughts? |
@kjellkvinge Thanks 👍 I agree it's hard to get it right. I've tried this solution but it still looks kinda weird. Maybe we should keep the width to 100% width and remove the |
ok, now trying a new approach. This was originally a PR for using full page width. Now it just adds scrollbars if the graph is to wide. This might solve both maintaining the same layout as the rest of the pages, and still usable even if there is a lot of parallel branches in the graph. |
@kjellkvinge I find this to be the best solution thus far. |
Not to much opinions out there :) Maybe I can provide some screenshots, or a live demo somewhere, so people can test without without rebuilding? |
@kjellkvinge Screenshots are good enough (I guess) |
LGTM |
Ref #599
This will use all available space to display the graph.