-
Notifications
You must be signed in to change notification settings - Fork 44
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
Number gutter misaligned after opening and closing Quick Edit #5
Comments
Actually, I thought about this and there is a really quick way you can test if Themes is being a brat. Remove Themes to see what happens... :) I will take a look also to see if I can give some ideas on how to fix the issue. |
I just tested it and when I do a fresh start and open Solarized, the document is all crazy. Resizing the window or just switching documents will cure the problem... Which tells me that there might be some refresh I can trigger internally to resolve the issue. Let me play around with it for a bit. |
Hi Lance, I have checked in a change to try to resolve the issue... Things worked correctly after the fix. Can you please give it a quick test? |
I still have the same problem. Can you do a CodeMirror refresh right after whatever event closes an inline editor? That is probably where you need it to solve this specific bug. |
Checked in fix. Quick edit works in the test I ran |
Miguel, you really should open this bug again. None of these fixes ever worked for me. I'll be glad to help out debugging, fixing, testing, or whatever. I'm sure it has something to do with issue a proper refresh around the time of opening or shutting the inline editor. Please open it again so we can address it. Lance. |
I don't think there is much I can do... The issue does happen without themes as well. Solarize sets paddings for the gutter which makes the issue a lot more prominent. I open a js file and near a define call I did a Quick Edit, and the paddings in the gutter flickered a bit and then the define call had the line number with the wrong indentation. Maybe you can help me verify this so that we can report this issue with brackets. |
I think the issue you outline above is different than this bug. I did try to repro the problem you show in your screenshot and was unable to do so. I might need more specific steps from you. Regardless, I am going to spend some time researching this bug today and I will let you know what I find out. |
Sure, a little research won't hurt. But the issue is that Solarized.css sets padding values for the gutter... The way codemirror computes width for gutters is affected by values in your css, which is really nice but can be fragile as we are currently seeing because order of operations now matter. You need all the stuff that affect the dimensions of the gutter loaded before documents are created. I haven't had time to look into how quick edit stuff does it magic, but it seems like it is creating a new codemirror instance inline and gutter dimensions are somehow affected negatively. Take a look at any padding quick edit specifies for the gutter... |
BTW, the issue I outline is a manifestation of the same issue; customizing gutters which is what Solarized.css is doing. |
I'm done with my initial research. I know what has to be done to solve the problem but I am still figuring out how to write the code. I agree with you that the initial problem is caused by the different Themes having different padding values in the gutter. However, the padding problem can be solved by calling refresh(), either in the Editor or directly from the CodeMirror object, just after the theme is applied. It looks like you tried to address this problem at the very end of your code:
but there is at least one other place it needs to be called as well. That's the easy part to fix. I implemented a fix with the setTimeout() and it does fix the problem, but using setTimeout() is a kludge and I have to fix that next. That's the difficult part. I have to solve a similar problem with my Cowsay extension, loading an external file asynchronously and responding to it when it finishes loading, so this will force me to finally do the async code I have been avoiding :). Hopefully, I will have a pull request for you soon. |
I had written code to refresh when the css resources were loaded but it didn't help the situation a whole lot with quick edit. I checked the changes in because they made more sense to known when to cause the refresh rather than blindly adding a setTimeout and hoping that I gave enough time. This does not really help inline widget in code mirror and really dread debugging code mirror and brackets more than I have to. So, a pull request would be really nice. |
Hey Lance, how is it going sir? Just checking to see if you had made any progress on what you had in mind. |
I am actually looking at it today :). I got distracted with the Column Ruler extension I am working on, but I made some good progress on it this week so this bug is next on my to do list. |
Miguel, this is my easy fix solution I have had done for a while with the setTimeout() hack in it. I haven't been able to figure out the correct asynchronous solution but I don't see any reason to delay fixing this bug. If I can figure out the better solution later, I will submit that. For now, this works. |
Fixed by #13 |
Repro Steps:
Observed Behavior:
All of the line numbers in the gutter, except for the currently selected line, are shifted to the right:
Expected Behavior:
All of the line numbers in the gutter should remain aligned:
The text was updated successfully, but these errors were encountered: