Skip to content
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

Color picker is very wide and cannot be resized #178670

Closed
DanTup opened this issue Mar 30, 2023 · 8 comments
Closed

Color picker is very wide and cannot be resized #178670

DanTup opened this issue Mar 30, 2023 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-color-picker Editor color picker widget issues

Comments

@DanTup
Copy link
Contributor

DanTup commented Mar 30, 2023

The colour picker seems to stretch across most of my monitor unnecessarily. This means the sliders down the side are quite far away:

Screenshot 2023-03-30 at 11 25 31

As far as I can tell, I can't resize it.

@aiday-mar
Copy link
Contributor

Hi, thanks for commenting, the resizing functionality has not appeared yet in VS Code Insiders. You can monitor the PR here: #176744

@aiday-mar aiday-mar added editor-color-picker Editor color picker widget issues bug Issue identified by VS Code Team member as probable bug labels Apr 6, 2023
@aiday-mar
Copy link
Contributor

aiday-mar commented Jul 13, 2023

The content hover can now be resized, and hence this issue will be closed.

@DanTup
Copy link
Contributor Author

DanTup commented Jul 13, 2023

@aiday-mar it seems like the new size is only saved sometimes.. It did save when I first reduced the width, but it doesn't seem to save further changes:

color.mp4

@aiday-mar
Copy link
Contributor

Hi thanks for your comment. Please have a look at the following issue which explains what you are seeing: #187904 and proposes the idea of persisting on a per token basis.

@DanTup
Copy link
Contributor Author

DanTup commented Jul 14, 2023

@aiday-mar can you clarify what you mean by "token" here? Do you mean for each type of popup (hover vs colour picker), or for specific tokens in the source code?

I think persisting the size of hover vs colour picker separately makes sense, but if the issue here is that I resized it smaller than some allowed minimum, I think perhaps the minimum is too large?

@aiday-mar
Copy link
Contributor

I mean for specific tokens in the source code. More specifically persisting the size of the content hover per word inside of the source code.

The color picker is in fact embedded within the content hover. It is possible to have for example diagnostic errors alongside a color picker within a hover hence I am not sure if we should persist these separately. This is different however for the standalone color picker which never appears alongside any other hover content.

You resized the content hover below the default maximum hence the maximum dimensions did not change and rendering happened as initially, within the constraints of the default maximum dimensions. We could change the default maximum dimensions - ie, make them smaller, but the root behavior would persist.

@DanTup
Copy link
Contributor Author

DanTup commented Jul 14, 2023

I mean for specific tokens in the source code. More specifically persisting the size of the content hover per word inside of the source code.

I'm not sure I understand the reason for this. If I resize my colour picker, I want it to be that size for all instances. I don't see value in having it per-word (especially since code changes and those would presumably quickly become stale).

You resized the content hover below the default maximum hence the maximum dimensions did not change and rendering happened as initially, within the constraints of the default maximum dimensions.

Do you mean maximum or minimum? I increased the height but reduced the width. If there is a minimum or maximum I violated in one axis, it would be nice to persist the other axis. It's not obvious what's happening with the current behaviour (or why I can't just have it exactly the size I'd like).

@aiday-mar
Copy link
Contributor

aiday-mar commented Jul 14, 2023

I'm not sure I understand the reason for this. If I resize my colour picker, I want it to be that size for all instances. I don't > see value in having it per-word (especially since code changes and those would presumably quickly become stale).

Indeed this is not a problem for the color picker which always has the same initial size. This feature would be more useful for the generic hover widget, into which the color picker is embedded. The content hovers, generally, are of different sizes, some can be small and some can be big. I see now why you mentioned you would like to persist the color picker size in a different manner to the generic hover. Currently the color picker is embedded into the content hover and can appear alongside other content (like diagnostics), therefore we should discuss with the team whether we would like to apply a different persisted size to a hover as soon as it contains a color picker.

Do you mean maximum or minimum? I increased the height but reduced the width. If there is a minimum or maximum I
violated in one axis, it would be nice to persist the other axis. It's not obvious what's happening with the current
behaviour (or why I can't just have it exactly the size I'd like).

I mean the maximum. More specifically going back to the issue here: #187904, the current maximum height or maximum width on initial render are defined as follows below. Here lastHeight and lastWidth are the last height and the last width you resized the content hover to. If lastHeight is smaller than the default, currentMaxHeight will remain unchanged. Hence the content hover will aim to maximally grow into the maximum size (and therefore will remain unchanged height-wise). The same goes for the width.

const currentMaxHeight = Math.max(this._editor.getLayoutInfo().height / 4, 250, lastHeight);
const currentMaxWidth = Math.max(this._editor.getLayoutInfo().width * 0.66, 500, lastWidth);

To explain it in another way, the content hover grows to its maximum size, which changes depending on your interaction with it (according to the equations above). That is why from rendering to rendering, the hover is, sometimes, not in the exact state as you left it before. This persistance method is in fact under discussion within the team. The idea that is proposed in the github issue above, is for each token to persist a dimension, its last resized dimension, which is reused when hovering on that token again. That would solve the problem of having unpredictable content hover sizes.

We could of course decide to always persist one hover dimension, the last resized dimensions, that would be applied to all content hovers independent of the location where they are spawned, but this would not work. Specifically because, as was mentioned, content hovers are of different sizes - some are big and some are small. You may in that case end up with a big content hover and little text inside. Therefore a more refined solution is needed. Of course in the case of the color picker, the situation is different because it just so happens that the color picker is always rendered with the same size inside of its container, the content hover.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug editor-color-picker Editor color picker widget issues
Projects
None yet
Development

No branches or pull requests

2 participants