Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Cannot resize JSLint panel or Search Result panel. #1122

Closed
RaymondLim opened this issue Jun 22, 2012 · 18 comments
Closed

Cannot resize JSLint panel or Search Result panel. #1122

RaymondLim opened this issue Jun 22, 2012 · 18 comments
Assignees

Comments

@RaymondLim
Copy link
Contributor

Open a file that has some JS Lint errors or do a Find-in-Files. When JSLint or Search panel shows up, try to resize the panel so that you can see more of its content.

Result: You can't.

@RaymondLim
Copy link
Contributor Author

Set it to medium priority since this is one of the reasons that I can't use Brackets for my everyday work.

@ghost ghost assigned ryanstewart Jun 28, 2012
@pthiess
Copy link
Contributor

pthiess commented Jun 28, 2012

Reviewed - The work you did in the side bar may apply here too. Please have a look.

@pthiess
Copy link
Contributor

pthiess commented Aug 14, 2012

Move to Backlog - probably a starter feature

@pthiess
Copy link
Contributor

pthiess commented Aug 22, 2012

@ryanstewart - I move this to the backlog Trello card #608.

@jbalsas
Copy link
Contributor

jbalsas commented Sep 3, 2012

Hi!

I've seen in this Trello card that this is marked as a starter bug although it is not in here.

I'd like to try this one out if that's okay. Other than the existing side panel resize, is there anything else to take into account?

Looking around, I've noticed the z-index definitions in styles/brackets_variables.less. Is there a document or some rules defining how indexes must be assigned? If it does, I haven't been able to find it in the wiki so far.

@redmunds
Copy link
Contributor

redmunds commented Sep 4, 2012

Chema, Yes, you are welcome to take on this one.

I think the intent here is to add functionality to allow the bottom panel to be resized vertically only. Resizing the width of the editor or app, or changing the side panel width, will resize the panel horizontally.

The z-indexes were defined in variables in brackets_variables.less for the sake of organization. We have not yet run into any conflicts, so that's why there's no documentation. But, by only allowing the height of the bottom panel to change, then you shouldn't need to change any z-indexes.

@jbalsas
Copy link
Contributor

jbalsas commented Sep 4, 2012

Thanks Randy, I didn't want to step on anyone's toes.

I've been already looking around and I have a couple of questions:

  1. The sidebar hides when double clicking on the resize handler. Should the bottom panels work the same way? Should this, in the case of the jslint panel trigger the "view > enable jslint" command ?
  2. The sidebar hides when it is dragged to a value under 10px. Should the bottom panels work the same way? In this case, both panels have a ".toolbar" div with the title which is 30px. Should this be the limit for hiding them, or should we keep it like the sidebar?
  3. The search panel has a close arrow on the top right side. Should we add one for the jslint panel as well?
  4. If one panel is opened and another one pops up, they may exceed the app height. Should we adjust the panels sizes somehow?

@redmunds
Copy link
Contributor

redmunds commented Sep 4, 2012

  1. Good point. The JSLint panel cannot be closed while JSLint is enabled and there are errors. Auto-disabling JSLint seems cryptic to me -- I think I'd prefer enforcing a minimum height, instead.
  2. Same answer as 1.
  3. No, the JSLint panel should not have a close button.
  4. Good catch. Yes, I think we'll need to enforce a minimum height on editor. This brings up another good point -- if there are 2 panels open and they are using maximum panels height, then it would be nice if increasing the height of 1 causes the other to decrease in height as you drag (otherwise you'd have to decrease height or close one before increasing height of the other).

@jbalsas
Copy link
Contributor

jbalsas commented Sep 8, 2012

Hi! I have pushed a rough implementation of this in https://github.com/jbalsas/brackets/tree/jslint-panel-resize

I stopped before doing any cleanup or any API changes because I'm not sure if this is what you'd want for Brackets. There are a some things that bother me about this:

  • Seeing this in action, I think it does not make lot of sense to have both panels visible at the same time taking space from the editor. Also, what will happen if the number of available bottom panels grow? Would you keep stacking them?
  • I think the solution you propose would cause some weird behaviors. For instance, if you try to increase the height of the top panel when the available height is maxed out, you'd see the panel grow downwards when you're moving the mouse in the opposite direction.
  • Having to resize other panels would require to tie them together and expose some new APIs, which maybe you don't want.
  • The resize action is already hard on performance. Having to resize several panels at the same time will make things worse. (On a side note, resizing the search panel degrades at the beginning as the number of results grow, but then stays steady. It doesn't look worse to resize the panel with 5000 results than with 500)

I could finish tweaking and cleaning this up, at least for now, if you think it's worth having this functionality.

For the long term, I'd suggest to group all the available bottom panels in one tabbed bottom panel. This module could expose some API to add/remove and enable/disable panels. It will also be the responsible for handling the resize and invoking some setHeight API on the available panels when required.

What do you think?

@redmunds
Copy link
Contributor

redmunds commented Sep 9, 2012

I think I agree that stacking panels is not a good solution. I was just thinking through how it works now to make sure current functionality doesn't get broken. It sounds to me like the tabbed panels is a better solution, but I am just 1 person on the team. I think it would be best to start a discussion thread on the brackets-dev forum (https://groups.google.com/forum/?fromgroups#!forum/brackets-dev) to reach a general consensus before you proceed.

@redmunds
Copy link
Contributor

redmunds commented Sep 9, 2012

Also, whether or not to have a close button on the JSLint panel would be another good topic to discuss in that thread.

@jbalsas
Copy link
Contributor

jbalsas commented Sep 14, 2012

A discussion about this is underway in the brackets-dev forum at https://groups.google.com/forum/?fromgroups=#!topic/brackets-dev/PqEbuhw7k6c.

It looks like bottom panels organization will change in the future. Should we push the changes to fix this issue in the meantime? Or is it better to keep it clean and just close it?

@redmunds
Copy link
Contributor

You are welcome to push any changes so we can take a look adn decide if it's the direction we want to go.

@pthiess
Copy link
Contributor

pthiess commented Sep 26, 2012

@jbalsas Thanks much for submitting your pull request. I marked this FIP and removed the Move to Backlog tag since you're working on it already. Cool!

@pthiess
Copy link
Contributor

pthiess commented Sep 26, 2012

Nominate to Sprint 15

@njx
Copy link
Contributor

njx commented Sep 28, 2012

Reassigning to Randy since he's reviewing.

@ghost ghost assigned redmunds Sep 28, 2012
redmunds added a commit that referenced this issue Oct 8, 2012
@redmunds
Copy link
Contributor

redmunds commented Oct 8, 2012

FBNC back to @RaymondLim

@ghost ghost assigned RaymondLim Oct 8, 2012
@RaymondLim
Copy link
Contributor Author

Fix verified.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants