Master chips lul #6480
Open
Master chips lul #6480
Conversation
This file contains hidden or 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
|
|
LucasLefevre
force-pushed
the
master-chips-lul
branch
4 times, most recently
from
c5bfa8c to
f3e75f5
Compare
hokolomopo
reviewed
May 27, 2025
| @@ -2951,7 +2951,7 @@ export const demoData = { | |||
| C6: "-445248", | |||
There was a problem hiding this comment.
Some functional notes:
- hover style of simple arrow (without chip) is worst than before IMO ? particularly on white background
- hover style of arrow/chip is set when hovering on the cell, not on the icon. And the rest of the cell isn't clickable
- whole chip should probably be clickable, not just the arrow ?
There was a problem hiding this comment.
- cannot use chip in "Values in range" ?
There was a problem hiding this comment.
- font size looks really small in the autocomplete dropdown
There was a problem hiding this comment.
- cannot change text color of chip item with dark background. Chip text color should not take priority over user-defined color. Nor cf colors.
There was a problem hiding this comment.
- when chip arrow is white on dark background, hover style is barely noticeable
| @@ -0,0 +1,5 @@ | |||
| .o-spreadsheet { | |||
There was a problem hiding this comment.
css won't work with my PR to put all the icons in the canvas. Actually we should discuss how both task will interact, because I don't think it'll be easy
| this.hoveredCellStore.row === this.props.cellPosition.row; | ||
| const shadowColor = darkenColor(style?.textColor || TEXT_BODY_MUTED, 0.2); | ||
| return cssPropertiesToCss({ | ||
| color: style?.textColor, |
There was a problem hiding this comment.
should probably darken the color rather than adding a shadow on hover
src/stores/grid_renderer_store.ts
Outdated
Comment on lines
601
to
604
| let style = { | ||
| ...this.getters.getCellComputedStyle(position), | ||
| ...this.getters.getDataValidationCellStyle(position), | ||
| }; |
There was a problem hiding this comment.
I don't like this. Every call to getCellComputedStyle is now wrong, except when we add getDataValidationCellStyle manually. Data validation style should be included in cell compute style.
LucasLefevre
force-pushed
the
master-chips-lul
branch
from
f3e75f5 to
37237a7
Compare
The hovered col/row will be used for data validation, not only tables. Task: 4800440
This commit moves the text coordinate computation to boxes. It's easier to test, decoupled from the canvas. They'll be needed to compute coordinates of data validation chips which will also be on the boxes Task: 4800440
I still see odoo employees use spreadsheets from competitors instead of using our own spreadsheet. In almost every single one of them, there are data validation chips. People want chips? We'll give them chips! This commit adds colors for dropdown data validation and it adds the "chip" display mode. Task: 4800440
LucasLefevre
force-pushed
the
master-chips-lul
branch
from
37237a7 to
3b1192b
Compare
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.
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist