Modernize the wibox.layout.grid module and add border support.
#3746
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Elv13
changed the title
wibox.layout.grid module and add border supports.wibox.layout.grid module and add border support.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3746 +/- ##
==========================================
+ Coverage 91.01% 91.05% +0.04%
==========================================
Files 901 909 +8
Lines 57566 58202 +636
==========================================
+ Hits 52392 52998 +606
- Misses 5174 5204 +30
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Aire-One
reviewed
Dec 22, 2022
lib/wibox/layout/grid.lua
Outdated
Comment on lines
983
to
994
| if type(val) == "table" and (val.inner or val.outer) then | ||
| self._private.border_color = { | ||
| inner = gcolor(val.inner), | ||
| outer = gcolor(val.outer), | ||
| } |
There was a problem hiding this comment.
If the use calls the set_border_color method with only one component in the table, the other has to be reset-ed?
Looks like a rebase leftover from an earlier iteration of the code.
This is already used in the `wibox.layout.manual` layout. It makes the widget easier to use. Previously, using the imperative syntax was necessary for most grids.
This is long overdue. A bit of historical context. The grid API is losely somewhat based on the old `radical` module, but was heavily improved by @getzze. That version had row_span and col_span. This made the way the previous implementation coded the border incompatible. I spent some time back then trying to bolt it back on, but the complexity is quite high and never made it work right. This commit goes in another direction. Rather than draw the border, it creates a mask where the border should *not* be, then bucket fill the widget. This is the equivalent of CSS `border-collapse`. It also support custom borders. This allows dashed lines and partial borders. The main use case will be to add border support to the calendar. It was previously possible to partially do it using custom cell painters, but was pretty hacky. Now that the calendar will deprecate the custom painters in favor of `widget_template`s, a more robust alternative was required. The drawback of this commit is obviously the added complexity to the most complex layout. This is why it adds many tests to cover the various corner cases.
* Stop using top level properties for vertical/horizontal * No abbreviations * Use height/width instead of size * Don't use methods where properties can be used
actionless
approved these changes
Mar 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is part of a larger branch I have been working on over the last month to implement
widget_templateeverywhere. When it came to the calendar, it has its own "fn_embed" API to implement "templates". I try to prevent regressions when deprecating things. In that case, it is very hard to implement a 1 pixel border using the template (0.5px usingwibox.container.margindoesn't look right). The annoying solution is to actually implementborder_widthin the grid. I gave up on it 5 years ago, but now I rewritten my previous attempt in a much saner way. Rather than try to draw the border, it clip areas where there should be no border and bucket-fill the remaining content.Obviously, table borders are always rather nasty code. This is no exception. There was already a very large number of math code in the grid, now there is more. However, after fighting the corner case, I think it work reasonably well now. Beside the calendar, I can see a lot of place where this code can be useful. A lot of code examples use their own half baked "tables" using the margin container. They are almost never aligned property. Now there is a "correct" way to fix this. This PR doesn't do it. It's large enough already.
The PR also rename a bunch of properties which used abbreviations. It also convert
local v, h = w:get_dimentsions()to therow_countandcolumn_countproperties, which is much more in line with other APIs.