Remove table format #2225
Remove table format #2225
Conversation
juliaroldi
requested review from
JiuqingSong,
Andres-CT98,
flyingbee2012,
BryanValverdeU,
ianeli1 and
vinguyen12
and removed request for
JiuqingSong
| @@ -8,14 +8,17 @@ const DARK_COLORS_LIGHTNESS = 20; | |||
| const BRIGHT_COLORS_LIGHTNESS = 80; | |||
| const White = '#ffffff'; | |||
| const Black = '#000000'; | |||
| const DEFAULT_COLORS = ['rgb(0,0,0', '#000000', '#ffffff', 'rgb(255, 255, 255)']; | |||
There was a problem hiding this comment.
Why need this?
It is possible user's default color is not black/white
There was a problem hiding this comment.
I think DEFAULT_COLORS is a bad name. The DEFAULT_COLORS is not the user default color, but the colors that we use when we adapt the text color.
There was a problem hiding this comment.
If we are removing the background color, we also need remove this textColor that we added before.
There was a problem hiding this comment.
Hmm, how can we know if this color is added by the code when we add background color, or added by user's manual action?
There was a problem hiding this comment.
Maybe check the color override flag from cell metadata. If there is color override flag, background color is added from table cell shade feature, then text color on table cell is also added there. Otherwise, the text color may be from original table. In that case, should we still change the color?
There was a problem hiding this comment.
And in this change do you also need to clear that flag?
There was a problem hiding this comment.
To avoid matching text and background colors, I checked if they were equal and also equal to the adaptive text color. I tried metadata approach and it did not work, because it erased the text color when I did these steps:
- Add a header row
- Change the header text color
- Delete the header row
- The text color was gone.
| if (block.blockType == 'Paragraph') { | ||
| if ( | ||
| block.segmentFormat?.textColor && | ||
| DEFAULT_COLORS.indexOf(block.segmentFormat?.textColor) > 0 |
| */ | ||
|
|
There was a problem hiding this comment.
remove this empty line
| */ | ||
| function shouldRemoveColor(textColor: string, cellBackgroundColor: string) { | ||
| if ( | ||
| ([White, 'rgb(255,255,255)'].indexOf(textColor) > -1 && |
There was a problem hiding this comment.
It seems you are checking both text color and background color to be white (or black). Are you sure about that?
It only returns true when black on black, or white on white.
There was a problem hiding this comment.
I changed to consider other bright or dark colors
When toggle table header, remove the text color that was adapted to appear in the header background color.
