-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Split the code around Draggable and DragTarget
#79784
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
Conversation
to improve developer happiness and lower cognitive load while working on parts of it.
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.
I think this is diverting from our styling. We tend to put relevant things in one file, and these classes are closely related. I suggest we do not proceed with this PR and should keep it as is.
|
to be hones i would find this sad. there are learnings about how to structure code for better developer happiness and ergonomy that are totally independent of the language used etc. and that in my experience make the life way better. having smaller, more conciese files is one of them. i would love to have more opinions on that or at least a hint which part of the official style guide supports what you wrote. |
|
this is a classic example of a change that does not take anything from you, but gives me something :-) |
|
test exempt: moving code with no semantic change |
|
I don't have a fundamental objection to reorganising code in general, but this particular reorganisation IMHO isn't ideal. For example, it makes DraggableState public, which we really don't want (states should be private — there are some unfortunate exceptions, but they pretty much all date from before we knew what we were doing). Similarly, having a file for a typedef is definitely not the prevailing style here. All of which is to say, I agree with @chunhtai. |
|
Yes. Not making state public absolutely makes sence and a file for a single typedef feels odd to me too. I moved code around rather rigorously to express in which direction i would like to go and completely shot over the target with that :-) |
|
My goal would be to have well cut files that are straight forward to navigate because what is in there and the name correlate perfectly and that are not so big that they overwhelm a normal persons "mental RAM". An obvios question about the current state would be: "what makes DragTarget so special that it is allowed to be the name giver for the file?" I as a developer would expect the file to be called draggable.dart for example. |
|
I have no problem renaming the file to draggable.dart, one thing i am not so sure is if this is going to break any existing app. If it does, it may not worth it to do it just for file name changes. |
|
The naming change alone is not worth it. That was just an example to express that the situation now is also not ideal. |
|
I think on the bottom line, you can separate files if it won't expose new API. It makes sense to have one file for one widget class. Things get tricky If there are two widgets that rely on common mixin/untility/enum. It can be either separated into three files, widget_a.dart, widget_b.dart, widget_base.dart; or both widgets are in the same file and keeps common classes private. To decide which one we go we need to see whether the common classes can be useful outside of the use case of widget_a and widget_b or they are needed to be exposed to use the API. If it does, then we should make it public and have three files. |
|
@prosac Just so I understand the motivation here better (I'm not super familiar with these files), is the desire coming from the files being too long (1000 lines today)? I could see a world where DragTarget and Draggable are in two separate files, with the rest of the classes distributed as needed to make that work, if that is possible without exposing any new APIs. Historically we have had the rule that when a file hits 1000 lines it's time to split it, so that would make sense to me. (We rarely apply this rule in practice.) |
That sounds like a plan. I will try that during the weekend. Regarding my motivation. I work mostly with ruby and in that world developer happiness and ergonomy are a big thing. Whenever I come across code bases in almost other languages the lack of interest for that really catches my eye and I really feel the difference. I remember also reading scientific stuff about the cognitive load that binds your thinking horse power when your brain is aware of too much stuff that is not needed for the thoughts that should be in focus when working on a thing. I will search my bookmarks and the rest of the web to find that and if I can find it post it here. |
|
I totally agree with your desire for developer happiness and ergonomy, I'm just not sure that splitting files helps one way or the other with that. To work on the drag-and-drop logic you need to know all the stuff in this file, regardless of whether it's in one file or several. If anything, having it all in one file can make it easier for someone to understand the scope of what they need to learn. |
that is also true for a folder or any other kind of sorting mechanism. Guys, it's getting late here. I go to bed now. It is a pleasure discussing these things with you! |
|
I don't think splitting files is necessarily better for a developer experience. For example, if I am dealing with text selection, my folder structure may look as follows: If the different things in that file got split up just because it's ~1,700 lines of code, where would I look? What are the classes that I need to care about? Which ones are private and only used within the scope of this functionality? On the other hand, I can click the Structure tab which is right below the Project tab and I get a similar list to jump to things: I know which things are private to just |
Important consideration and somehow related to what I would want to achieve. I want make sure that things I do not have to care about are not in my field of view when working on things in the neighborhood. I want to reduce visual clutter that leads to more cognitive load. Code itself is a user interface. The pieces that make up the little sub domain of dragging and dropping are not hidden away, nor they are spread all over the project in my proposal. They are just around the corner. In my opinion that does not complicate things, but makes is more easy to work with.
One feature of one editor, also when it is the flagship editor is something i would not weight too high especially when another features of the same editor help a lot more with code navigation: like "go to definition" and fuzzy finding etc... Nothing against the structure view. It is neat and helps with overview... when the files are so large that you actually need it ;-)
That is the important part and you are totally right, but it does not necessarily require you to have everything in one file. |
|
From the discussion it sounds like we decided against splitting the logic this way. I am closing this PR for now then. If there's anything else to do, we can discuss it in an issue. |
|
I hope i will find some time to go on with my project that i am using flutter for. maybe then i will also think further about this. |


This PR depicts a proposal to split up the code around
DraggableandDragTargetetc. to improve developer happiness and lower cognitive load wile working on it.I want to propose this because I felt the need for it when working on https://github.com/flutter/flutter/pull/73143/files.
Please look at it very differentiated: It is just about restructuring who the code is organized into files and get opinions about this. It does not claim completeness or absolute correctness on where exactly to slice.
Opinions please! :-)
Maybe you @goderbauer, @chunhtai and @Piinks might want to have a look, as you have all looked at my PR that I mentioned.