Conversation
| this._enforceSingleSelection = true; | ||
| this._filterSetValue = true; | ||
| this._minWidth = 390; | ||
| this._minWidth = 410; |
There was a problem hiding this comment.
I saw in the git blame that this 390 value was set explicitly to provide enough room with scroll bars. I'm not sure why, but it's insufficient in our case. We still get wrapping at 390. If we're concerned about the size increase here, I could explore why that is.
There was a problem hiding this comment.
Worth noting that 410 might only be sufficient for English. We're not necessarily aiming to make it fit for all languages without wrapping, but we at least would like it to not wrap in English, and hopefully some other languages too. We know there will be languages where wrapping will likely happen.
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| selected: this.selected, | ||
| text: this.text, | ||
| valueText: this.valueText, | ||
| minWidth: this._minWidth, |
There was a problem hiding this comment.
This adds the width into the object that is ultimately propagated up in the d2l-filter-dimension-data-change event
| if (dimension.searchValue || dimension.searchType === 'manual') shouldSearch = true; | ||
| shouldRecount = true; | ||
| this._activeFiltersSubscribers.updateSubscribers(); | ||
| newValue.forEach((field) => this._minWidth = field.minWidth ? field.minWidth : this._minWidth); |
There was a problem hiding this comment.
Now that the width is available in the details, we'll apply it.
Note that we're not doing any Math.min math here- we're just setting it with no regard for the prior value. This is consistent with the current implementation- I'm not sure why that's the case, but I'm making the assumption that there's a good reason for that, though I wonder if it's also just because there happens to only ever be one width that differs from the base value.
Let me know if that was just an oversight I should fix while I'm here.
There was a problem hiding this comment.
We've had some discussion about this offline, and I think we're in agreement that we should apply a fix here. However, @bearfriend made a point about probably needing to do a max min-width, since we think the current fix would be relying on this being the last one (not overridden by a smaller min-width).
There was a problem hiding this comment.
Makes sense- I figured it was just an oversight due to the coincidence of there only being one min width to apply ATM.
There was a problem hiding this comment.
I think we're in agreement that we should apply a fix here.
Anything I can do to help us be sure we should apply a fix here? 😸 Should we book a chat perhaps? Or will y'all figure it out on your end?
There was a problem hiding this comment.
Makes sense- I figured it was just an oversight due to the coincidence of there only being one min width to apply ATM.
Quite possible.
Anything I can do to help us be sure we should apply a fix here? 😸 Should we book a chat perhaps? Or will y'all figure it out on your end?
I'll create a ticket for this. We'll prioritize it against our other tickets, but if you want it right away, we can chat about contributing a fix.
There was a problem hiding this comment.
but if you want it right away, we can chat about contributing a fix.
We'd love to do the work for this on our end if possible- not necessarily a critical issue for our team, but we have the bandwidth to pick it up.
2 participants
https://desire2learn.atlassian.net/browse/NTNGL-5248?atlOrigin=eyJpIjoiYWI2ZTc5N2ZjOTg2NDMxNTkxMmY2MjlmMjE1MTI0MzQiLCJwIjoiaiJ9
Opening this as a draft to see if I'm on the right track here before polishing it up, let me know what you think.
Problem
We were noticing that our date fields were reverting to the stacked/block form depending on the length of the column title rather than defaulting to a minimum width that would fit the dimensions in a row.
Recording.2026-03-19.132950.mp4
We don't like the UX/look of the stacked controls, so we wanted to ensure that they showed up in a line wherever possible.
Solution
While exploring with breakpoints, I noticed that updates to minimum width values within dimensions sets were not being propagated up to the filter like other values were. Then, since our column data arrived async after the filters are opened, by the time we had the
minWidthdata the dimension data was no longer being updated. As a result, theminWidthvalue supplied by thefilter-dimension-set-date-time-range-valuewas being ignored.In this PR, I've updated the dimension update code to also propagate the
minWidthas a value up to the filter, and I've added a line in the filter to use that value to set its own minimum width.Other details of the solution have been written inline.
Demo
Recording.2026-03-19.132843.mp4