CodeEdit
CodeEdit copied to clipboard
CodeEditApp
[WIP] Fix Source Control UI Styling Issues
Description
Fix the remaining UI styling issues per #638 and based on the original design prototype of CodeEdit. (also fix a little formatting issue)
This PR is still a WIP as the styling of changed files List has animation glitches. Glitches detail is in the comment here and here.
Related Issue
- #637
- #638
Checklist
- [x] I read and understood the contributing guide as well as the code of conduct
- [ ] My changes generate no new warnings
- [ ] My code builds and runs on my machine
- [ ] I documented my code
- [ ] Review requested
Screenshots
To be added.
@lilingxi01 : As you can see from the video, when you switch to select one element to another, it has a strange effect on the icon.
https://user-images.githubusercontent.com/20476002/170878707-9260636b-0356-4f88-9ce2-76283373a0e3.mov
@Angelk90 : Thanks for catching! This is a known issue that I am working on right now. (It seems like there is no solution in pure SwiftUI so I might need some works on AppKit to resolve this issue).
@lilingxi01 : Strange behavior.
You could also implement the possibility to change the type of structure represented.
For example with a tree like this on github or how vscode goes.
It might be better all-together to switch to NSOutlineView same as how it's done with the Project Navigator. This would be way more flexible than a SwiftUI List
It might be better all-together to switch to
NSOutlineViewsame as how it's done with the Project Navigator. This would be way more flexible than a SwiftUIList
Not to mention the performance advantages when using a tree structure with many nested items which is the main reason I chose to go with NSOutlineView for the Project Navigator in the first place.
It might be better all-together to switch to
NSOutlineViewsame as how it's done with the Project Navigator. This would be way more flexible than a SwiftUIListNot to mention the performance advantages when using a tree structure with many nested items which is the main reason I chose to go with
NSOutlineViewfor the Project Navigator in the first place.
The thing is it's not nested views it's a list, it's no where near that of project navigator only the repository tab would really require it.
@nanashili as you can see above a tree view was requested. And depending on how many files where changed this can be very performance hitting (e.g. when doing a lot of refactoring at once). Therefore I think it's better to future-proof by choosing a way we know works well.
@nanashili as you can see above a tree view was requested. And depending on how many files where changed this can be very performance hitting (e.g. when doing a lot of refactoring at once). Therefore I think it's better to future-proof by choosing a way we know works well.
And as I told @Angelk90 it depends on if git allows it.... Because there are 2 options currently available. Show all files ore show file and the main folder only. Only reason github has that kind of view is because it's cloud based and the files are being uploaded to them everytime and they check the main directory of it. If git had something like github all applications making use of git would be using that file structure even githubs own application.
@lukepistrol don't get me wrong you can do something exactly like githubs structure by making extra calls to the folder at hand. But do we really wanna do that and impact performance for a few calls?
For now, the issue is that, when we are overriding the foreground color for SwiftUI List (because we want the icon to be colorful), the selection state cannot be reflected correctly. This is why I am trying to find another solution like NSOutlineView.
https://user-images.githubusercontent.com/36816148/170881152-4acc020e-a633-44fb-bb14-f754d2166d4a.mp4
@nanashili It's "just" choosing an efficient algorithm to do the parsing of the tree structure depending on the file path. And I for myself like to have a tree structure available for inspecting changed files.
@nanashili It's "just" choosing an efficient algorithm to do the parsing of the tree structure depending on the file path. And I for myself like to have a tree structure available for inspecting changed files.
Well then extra calls is what you have to do and someone has to implement it because I'm to busy rn and my next worries is building the repository view of source control.
For now, the issue is that, when we are overriding the foreground color for SwiftUI
List(because we want the icon to be colorful), the selection state cannot be reflected correctly. This is why I am trying to find another solution likeNSOutlineView.
Yeah we had this problem previously when we used List in the project navigator. I tried to solve this for hours but ultimately ended up implementing the whole thing using NSOutlineView which is a sub-class of NSTableView. Have a look at the Project Navigator source code if you need inspiration.
@nanashili Please don't feel offended every time someone has a different opinion than you on a topic. I just posted another possibility which would allow for further features with performance improvements. Nobody asked you specifically to implement the changes. It was just an idea.
@nanashili Please don't feel offended every time someone has a different opinion than you on a topic. I just posted another possibility which would allow for further features with performance improvements. Nobody asked you specifically to implement the changes. It was just an idea.
I'm not offended, opinions should be given and that's a fact. Just I don't currently have time to work on it, since I do mostly git related components and no I'm not complaining i prefer to do the git above others.
For now, the issue is that, when we are overriding the foreground color for SwiftUI
List(because we want the icon to be colorful), the selection state cannot be reflected correctly. This is why I am trying to find another solution likeNSOutlineView.Yeah we had this problem previously when we used
Listin the project navigator. I tried to solve this for hours but ultimately ended up implementing the whole thing usingNSOutlineViewwhich is a sub-class ofNSTableView. Have a look at the Project Navigator source code if you need inspiration.
I did have a look at it when building out the changes view and it was mostly suited for tree views which in my eyes at the current time when developing the list for the changes wasn't necessary since it doesn't have any tree views to build/show... I would ultimately have to use TableView for the repo view as it relies heavy on a tree view.
@lukepistrol : He's right @nanashili , to implement the tree view. First of all you have to take the path of each file and put the files that are in the same dir or under dir in the same array (if we can say so).
So it takes a pre-rendering operation, obviously at a cost. Not everyone likes tree view, in fact most IDEs don't use it.
As I mentioned already: NSOutlineView is a subclass of NSTableView. Therefore it can behave like a normal "List" as well as an outline view for tree-structured content. Since List in SwiftUI is very limited both in styling as well as performance regards it would be better to rely on a more mature framework like NSOutlineView or NSTableView in this regard. Don't get me wrong, SwiftUI is a nice tool for less complex UI but once you need a certain complexity it just doesn't fit yet. This is the reason we rely on AppKit classes in components like the Project Navigator or the Editor.
Also the issue with the flashing icon cannot be solved in SwiftUI - at least for now.
@lukepistrol : He's right @nanashili , to implement the tree view. First of all you have to take the path of each file and put the files that are in the same dir or under dir in the same array (if we can say so).
So it takes a pre-rendering operation, obviously at a cost. Not everyone likes tree view, in fact most IDEs don't use it.
Who's right now @Angelk90 ?
As I mentioned already:
NSOutlineViewis a subclass ofNSTableView. Therefore it can behave like a normal "List" as well as an outline view for tree-structured content. SinceListin SwiftUI is very limited both in styling as well as performance regards it would be better to rely on a more mature framework likeNSOutlineVieworNSTableViewin this regard. Don't get me wrong, SwiftUI is a nice tool for less complex UI but once you need a certain complexity it just doesn't fit yet. This is the reason we rely on AppKit classes in components like the Project Navigator or the Editor.Also the issue with the flashing icon cannot be solved in SwiftUI - at least for now.
That i can agree with at the current moment List is limited to what it can do.
@lukepistrol : However, there is a problem that I do not know if you are aware of it or you have currently left to fix it later.
Consider the following setting, you have an open file that has not yet been modified, so it does not appear in changed files. You have on the left the version control open which tells you which files have been changed.
Now edit the open file and save it.
The open file that has been modified should automatically appear in the version control, which does not happen.
To make it appear, you have to click on another item in the left menu in order for it to be rendered.
https://user-images.githubusercontent.com/20476002/170882733-3ddcdf8f-04eb-4067-91cc-7c433900d03a.mov
@Angelk90 Seems to be a problem of the underlying data structure not observing changes and instead just reloading on appear. But I don't know how it is implemented. I think @nanashili implemented it.
@lukepistrol : However, there is a problem that I do not know if you are aware of it or you have currently left to fix it later.
Consider the following setting, you have an open file that has not yet been modified, so it does not appear in changed files. You have on the left the version control open which tells you which files have been changed.
Now edit the open file and save it.
The open file that has been modified should automatically appear in the version control, which does not happen.
To make it appear, you have to click on another item in the left menu in order for it to be rendered.
https://user-images.githubusercontent.com/20476002/170882733-3ddcdf8f-04eb-4067-91cc-7c433900d03a.mov
Could be the it's not observing properly or another option is it would need to be implemented in the editor view to make a call to update the view making a call to get the latest changes from the git.
@lukepistrol : I only find the problems. ;)
However if I modify the file on codeEdit, I save the file and then look on xcode if the modification appears, it is not given to me.
@lukepistrol : However if I modify the file on codeEdit, I save the file and then look on xcode if the modification appears, it is not given to me.
Xcode sucks with handling it, i think they might have their own built system.
@nanashili , @lukepistrol : I did the same test using webstorm, it works great there. I modify the file from CodeEdit, as soon as the focus is on the program (webstorm), the list is updated.
https://user-images.githubusercontent.com/20476002/170883756-39397adb-8af0-4fd4-bf13-3f2e3206a13d.mov
@nanashili , @lukepistrol : I did the same test using webstorm, it works great there. I modify the file from CodeEdit, as soon as the focus is on the program (webstorm), the list is updated.
https://user-images.githubusercontent.com/20476002/170883756-39397adb-8af0-4fd4-bf13-3f2e3206a13d.mov
There is multiple ways we fix this issue, it's not that big of an issue.
I agree with @lukepistrol especially because NSOutlineView is a subclass of NSTableView. @nanashili and @Angelk90 as mentioned, we should be thinking about the future here.
I agree with @lukepistrol especially because NSOutlineView is a subclass of NSTableView. @nanashili and @Angelk90 as mentioned, we should be thinking about the future here.
As I told @lukepistrol at the time i didn't see a need for it because building something like github tree view would require more calls than you think. So list was a given since changes doesn't use tree views.... And list can take up to 2000 items before hitting a performance point which almost no dev will ever make changes to 2000 files.
Thanks for discussion here. Your opinions are really helpful, and I will try implementing a new List using NSTableView (similar to how our NSOutlineView is implemented) once I get to my desk.
@austincondiff : I have not finished wrapping up this part yet. A bit busy this week, so I will finish this PR this weekend.
@lilingxi01 That last comment was made a month ago. Can we get this one finished up? I don't want it to get too stale. Do you have any blockers?