Increasing usability and design of files diff. #413
No reviewers
Labels
No Label
Priority-high
Accepting-merge-requests
API
API-dependency
Backport
Blocked
Brainstorming
Breaking
Bug
Changelog
CI
Cleanup
Confirmed
Discussion
Documentation
Duplicate
Enhancement
External-dependecy
F-droid
Feature
Google-play
Improvement
Invalid
Investigate
LGTM-done
LGTM-need
Long-term
Major-release
Minor-release
Needs-cleanup
Needs-feedback
Needs-help
Priority-critical
Priority-low
Priority-medium
Question
Ready
Refactor
Regression
Release
Repository
Security
Suggestion
Support
Testing
Translation
UI/UX
Upstream
Website
WIP
No Milestone
No Assignees
3 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: gitnex/GitNex#413
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch ":diff-cleaner"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #397
Todo
Include code commenting
(To cite code, click on the line(s) you want to cite and (if you're ready) long press on any already cited line.)
Support dark mode.
Summary
[WIP] Increasing usability and design of files diff.to Increasing usability and design of files diff.@ -6,6 +6,7 @@
<dimen name="tooltipCornor">5dp</dimen>
<dimen name="fab_margin">16dp</dimen>
<dimen name="close_size">23dp</dimen>
Please retain the original size, as 23 is too tiny on screen with higher resolution.
Also better name would be
close_button_size
.Really? It looks absolutly fine in your screenshot.
That is not high resolution phone.
Also the size was set to match the back button, hamburger menu etc. Compare to them this one at this size is small now.
I have just did a quick check and maybe 30 is a bit big. 26 does match the rest. A sweet spot.
Sounds great.
@ -118,6 +118,8 @@
<string name="orgCreatedError">Something went wrong, please try again</string>
<string name="orgExistsError">Organization already exists</string>
<string name="binaryFileError">Binary files cannot be shown.</string>
Binary files cannot be shown.
could beBinary files are not supported yet
Here are the actual comments:
Use the default fonts as used by the app.
Padding should be added to left and right as it really touches the screen edges.
How does the citation work? I clicked few lines, and hold left mouse button and it crashed the app.
If file occupy less space than visible screen size, the layout should be at the top. See screenshot.
Please retain the old filename for
FilesDiffAdapter.java
.You removed recyclerview for listview, any benefits?
Please use colors from colors.xml.
As we are on it, it would be nice to have proper color scheme so different themes can have nice look. It's easy, just the backgrounds and colors need to be adjusted.
This is intended and would'nt fit otherwise.
Monospace is used for source code, because it makes it much easier to read.
I may have a look at it, but i think too much padding may be unwanted, because the code itself often includes much empty space.
This shouldnt happen. Are you able to send me any stacktraces and additional information (repo, pr, etc.) for further investigation?
This was not intended and will be fixed.
Please scroll through the diff in the current release. Some textviews will be duplicated and are placed in positions, where they should'nt be. Thats the reason why i am using a listview in this specific case.
I will do that.
Ok. Hope @6543 is also ok with keeping this.
Remember some phones have screen egdes, adding padding just occupy the screen in front. Also it just does not look nice that way either by touching the phone edges. Looks rough.
Here
It could be a flaw in the implentation not necessarly recyclerview issue. But as you already used listview, it's ok now.
Also from my screenshot above, the background color should take whole line into account like how it was in previous implementation or how it look like in web.
@mmarif I have addressed many of your concerns in a new commit (colors are following). Are you able to test, if citation works now?
So far so good. Noticed few things which can be addressed along the way.
Citation works now, but if I cite and go to the reply section the start cursor is inside the code blocks(2nd line). Would be nice to have it in 1st line or below the code block.
The line background colors I guess you are still working on.
id/headerFileName
textview,id/header
and View colors are still hardcoded.This one is more generic issue though not related to this PR.
7dae652bf6
@mmarif Colors are still work in progress..
Seems like you guys like switch more than if. 😆
I'm ok with that but we will have some rewirte if we got a better api
for the collor: It would be good if the green woud be litle more green and the red more red
It also would be good to put colors into the theme so it changes based on witch theme you have -> white background on dark mode is - uggly
also can we add a divider between code blocks in same file - drawed suggestion (see atfachment)
ok attachments are broken so ..:
I am for keeping it the same as the web ui. Last time was like that.
Maybe in another PR?
I'm fine with that too
Great, but let me still have a look at the attachments issue @6543 had. Can you tell me which pull request you tried to access and maybe some hints for reproduction?
I feel he is referring to gitea.com attachements which are kinda broken after update.
@mmarif yes
@opyale pleace "update" this branch - I cant since its portected
@6543 Done.
Increasing usability and design of files diff.to [WIP] Increasing usability and design of files diff.@mmarif @6543 Added support for dark mode.
[WIP] Increasing usability and design of files diff.to Increasing usability and design of files diff.@opyale can you "Update" this branch ?
@6543 Did it already.
I'll have a look at the crashes after VersionRefactor
@mmarif @6543 I would suggest also targeting the 2.5.1 milestone, because these changes do also include minor bug fixes (like code duplication).
@code dedublication is no bug.
If you fixed what are those?
@6543 Duplication and random scrambling of code should be considered as a bug. I've already sent you a screenshot where XML glitched into some java code.
Seems this is already settled with another PR and that's how it has to be done. This PR even has fixes cannot be part of minor bug fix releases.
@ -58,3 +57,3 @@
android:id="@+id/listView"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:background="?attr/primaryBackgroundColor"
Please keep this and remove
android:background="@color/white"
for theme changes. It will also remove the white background in dark theme.Oops. I did also notice this, but forgot about it...
Nice work @opyale
Three things I noticed can be improved(one I mentioned before).
Check the screenshot, the white background. It should be the color background used for that specific line(gree, red etc) extend to the end of the line.
For BIN(images etc) the green, red(small change color indicators) should be hidden.
This is optional but nice to have if you have time. The spacing is less and the files looks very cluttered. I would give them some breathing space between each file. If not possible right now, I will look into it after merge this PR.
https://cloud.swatian.com/s/QtJWYbCCg2rQLZ4
After fixing 1, 2 and the background for listview, we are ready to merge this.
@mmarif I have addressed (almost) all of your concerns in my last commits.
Honestly, the goal was to actually make everything more compact and "cluttered", because especially when there are many lines of code and many inidividual files, space on the users screen gets really valueable (and especially on small screens).
We will discuss this another time. You can skip this.