feat: #30 Allow for variable-width columns and more than two columns #32
Conversation
andrew-polk
force-pushed
the
VariableColumnWidth
branch
from
48a1727 to
da5ee8b
Compare
| <div className='notion-spacer' />`; | ||
| } | ||
|
|
||
| // This is a hack. |
| // So we use 'notion-client' which uses the unofficial API. | ||
| // Once the official API gives us access to the format information, we can remove this | ||
| // and the 'notion-client' dependency. | ||
| async function getStyleInJsxFormat(blockId: string): Promise<string> { |
There was a problem hiding this comment.
This is fine, but consider if it would be cleaner to just have this as getColumnWidthPixels() and then have
style=`width=${widthInPixels}px`
| "\n\n" | ||
| )}\n\n</div>`; | ||
| )}\n\n</div> | ||
| <div className='notion-spacer' />`; |
andrew-polk
force-pushed
the
VariableColumnWidth
branch
from
da5ee8b to
91f7ad9
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @hatton)
src/transformers/ColumnTransformer.ts line 32 at r1 (raw file):
Previously, hatton (John Hatton) wrote…
Could use a comment
Done.
src/transformers/ColumnTransformer.ts line 35 at r1 (raw file):
Previously, hatton (John Hatton) wrote…
I don't think this is a hack
Done.
src/transformers/ColumnTransformer.ts line 40 at r1 (raw file):
Previously, hatton (John Hatton) wrote…
This is fine, but consider if it would be cleaner to just have this as
getColumnWidthPixels()and then havestyle=`width=${widthInPixels}px`
We want it as a percentage so it flexes with the size of the window like Notion does.
But I took your point about returning the width rather than the whole style.
|
Converting to draft because running this on bloom-docs revealed issues. |
andrew-polk
changed the title
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @andrew-polk)
src/transformers/ColumnTransformer.ts line 40 at r1 (raw file):
Previously, andrew-polk wrote…
We want it as a percentage so it flexes with the size of the window like Notion does.
But I took your point about returning the width rather than the whole style.
Oh, sorry, and before it was a just a number, but now it is a CSS expression, so the simplification benefit mostly went away :-) Ah well.
Ok, the problem is that the css changes have to be copied to the client apps.... which makes this somewhat of a breaking change. |
BREAKING CHANGE: Makes changes to the expected css for formatting columns. So clients must make those changes to their respective css files to get the desired results.
andrew-polk
force-pushed
the
VariableColumnWidth
branch
from
91f7ad9 to
0ba29b6
Compare
This change is
BREAKING CHANGE: Makes changes to the expected css for formatting columns. So clients must make those changes to their respective css files to get the desired results.