-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(component): add column and section #46
Conversation
add column component resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
add section component resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fixed email component with updated tags resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix email stories with the updated component structure resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
add missing stories for column component resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
add stories for section component resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix storybook.yml script to only deploy when push to main branch Signed-off-by: Niloy Sikdar <[email protected]>
Seems you assume users will use Email ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My pov:
Email
will be the outermost component with the div -> table
structure
If we are creating the Column
component, then it should be something that will wrap its children vertically, which basically means, all of the children which are inside the Column
should be aligned to create a column. For this purpose, I have taken tbody
which can wrap our tr
s to form the vertical alignment.
Now Section
will work like each row, which means users can have multiple sections inside a column, and all of the sections then will be placed vertically one after each other. For the Section
component, I have taken tr
.
Now we can include one or more components inside one Section
. Components will be wrapped with td
.
This is the same widget tree structure that Flutter also follows.
So our overall structure will look like this:
<!-- Email -->
<div>
<table>
<!-- Column -->
<tbody>
<!-- Section -->
<tr>
<td>This is 1</td>
</tr>
<!-- Section -->
<!-- Section -->
<tr>
<td>This is 2</td>
<td>This is 3</td>
</tr>
<!-- Section -->
</tbody>
<!-- Column -->
</table>
</div>
<!-- Email -->
But this can have some styling issues if we don't maintain proper items count for the sections in the right way, that's why I also thought of multiple table
nesting, but wasn't quite sure about that approach, that's why wanted to discuss with you about these approaches and texted you regarding the same @agentmilindu
If we follow these rules, then it won't work. <tr>
<td>
<Element 1/>
<Element 2/>
<Element 3/>
<td />
<tr /> Now in this case, |
…o feat/39-add-component-column-and-section
fix the structure of Email, Section and Column components resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix Email stories with the new structure resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix Section stories with new structure resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix Column stories resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
export props for components from the library resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix Email component with classes resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix types using Record and add styles to the prop for Email component resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix types to use Record and add styles props for Column component fix stories for Column with styles resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
fix types with extends BaseStyleProp and adding classes to the props list resolves #39 Signed-off-by: Niloy Sikdar <[email protected]>
d23db71
to
f8368a7
Compare
# [1.1.0](v1.0.1...v1.1.0) (2022-07-05) ### Features * **component:** add button component ([#47](#47)) ([c846451](c846451)), closes [#40](#40) [#40](#40) [#40](#40) [#40](#40) * **component:** add column and section ([#46](#46)) ([c89ad5e](c89ad5e)), closes [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39) [#39](#39)
This Pull Request covers:
Column
andSection
Resolves #39
Signed-off-by: Niloy Sikdar [email protected]