Skip to content

Conversation

sronveaux
Copy link
Collaborator

This PR was written first to try and address #439 giving to the Geocoder combobox a min and max width which should give it a behaviour close to what could be observed in Vuetify2 / Wegue v1.x. Those values can be adapted though.

At the same time, a bug where the header title gets truncated even when there is space left in the toolbar is addressed. This was reported to Vuetify some time ago. Answer was only given more than a year later in another similar bug report.

@chrismayer
Copy link
Collaborator

Thanks for this @sronveaux. I think removing <v-spacer></v-spacer> is a must since this is a fix.

Handling width, min-width, flex, etc. can become tricky as the discussion in #439 shows. I'd vote as a minimum to allow to configure width and min-width as props, so they could be set optionally in the module configuration.

:width="width"
:min-width="minWidth"

Without setting them the UI also looks quite nice IMHO:

image

So this would be a minimal approach by exposing Vuetify properties. Anything further regarding to a flex like behavior I am unsure, if we should implement that on Wegue level. But this we could discuss then in #439.

@sronveaux
Copy link
Collaborator Author

Hi @chrismayer and thanks for your comment,

I agree with you except I'd expose width, min-width and max-width personally.

I also think playing with the flex behaviour can become 'dangerous' as it could conflict with Vuetify internal workings. That's the reason why I posted a hack that I was not in favour of including in the PR in the end. But if this helps someone...

Last question is which values should be set as default. I just did a quick test and saw that, opposite to CSS rules, Vuetify currently makes width take precedence over min-width and max-width.
For sure they were set in an empiric manner but the values for min and max I've selected for this PR are the ones which closely match Vuetify2 behaviour. I would definitely use them as default for this reason and that way, people would either have the opportunity to define their own min and max or fixed-width value.

If you agree with that, I'll amend the PR in the following days. Just tell me what you think about it...

@chrismayer
Copy link
Collaborator

I agree with you except I'd expose width, min-width and max-width personally.

So you mean, you would not expose them? So what would be the recommended way for devs to change the width of the UI? Via CSS?

@sronveaux
Copy link
Collaborator Author

No no no ! I would definitely expose them ! But instead of exposing two properties (width and min-width) I would also add max-width !

@chrismayer
Copy link
Collaborator

No no no ! I would definitely expose them ! But instead of exposing two properties (width and min-width) I would also add max-width !

Oh perfect, then we are on the same page. I am also fine with setting defaults to come close to old v2 behavior. Thanks in advance for your input.

@sronveaux
Copy link
Collaborator Author

sronveaux commented May 12, 2025

O.K., PR is amended following preceding discussion with @chrismayer.

There is a difference though, the fast tests I did had something wrong, in the end (and it's better like that !) Vuetify follows official CSS rules and width is superseded by min-width and max-width. However, the PR implements what was discussed:

  1. Default behaviour is to have minWidth and maxWidth defined by default with values which closely match what was observed with Vuetify2.
  2. The user can override those defaults in the configuration file if he wants.
  3. The user can also set a fixed width value which takes precedence over minWidth and maxWidth.

Documentation is amended and unit tests have been added too.

@chrismayer
Copy link
Collaborator

Thanks for your ongoing work on this @sronveaux. I am +1 to merge this. If we want to to further adaptations they should be done in separate PRs.

@sronveaux
Copy link
Collaborator Author

Thanks @chrismayer, exactly what I wrote in #439. Let's see where the discussion will lead and whether we'll go further...
I've given a proposal there even though I'm not in favor of implementing it but I'll follow what happens in this thread and reply if requested...

@sronveaux sronveaux merged commit 198ba78 into wegue-oss:master May 13, 2025
1 check passed
@sronveaux sronveaux deleted the fix-geocoder-width branch May 13, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants