feat(footer): add component #505
Conversation
Codecov Report
@@ Coverage Diff @@
## master #505 +/- ##
==========================================
- Coverage 99.91% 99.79% -0.12%
==========================================
Files 183 189 +6
Lines 2453 2499 +46
Branches 604 614 +10
==========================================
+ Hits 2451 2494 +43
- Misses 2 5 +3
Continue to review full report at Codecov.
|
|
the |
|
@unix @ofekashery any review? |
|
|
||
| <style jsx>{` | ||
| div{ | ||
| margin-bottom:20px; |
There was a problem hiding this comment.
It is recommended to use layout for layout.
There was a problem hiding this comment.
what does this mean exactly?
| ...props | ||
| }) => { | ||
| const theme = useTheme() | ||
| const { breakPoint } = useFooterContext() |
There was a problem hiding this comment.
We have hooks on breakpoints.
But we can't use the hooks directly internally, this may lead to inconsistent data when rendering on the server-side, please refer here: #258
Referring to the design of the Grid component, we need to integrate the breakpoints in the style.
| children, | ||
| ...props | ||
| }) => { | ||
| return <div {...props}>{children}</div> |
There was a problem hiding this comment.
We can replace it with Footer.Group, there is no need to add another component, if the user adds a nested structure, we just parse the structure.
components/footer/footer-link.tsx
Outdated
| li:before { | ||
| content: '' !important; | ||
| } | ||
| li :global(a) { |
There was a problem hiding this comment.
We need to be compatible with text out of alignment, centered or not, etc.
| maxWidth, | ||
| breakPoint, | ||
| ...props | ||
| }) => { |
There was a problem hiding this comment.
For Footer, I guess there are also users who might want to hover at the bottom of the page.
| subFooter?: string | React.ReactNode | ||
| ariaLabel?: string | ||
| maxWidth?: string | ||
| breakPoint?: string |
There was a problem hiding this comment.
Maybe we also need to be compatible with the imperative API.
<Footer data={[]} />
There was a problem hiding this comment.
Would try to work on this!
| light, | ||
| ariaLabel, | ||
| children, | ||
| subFooter, |
There was a problem hiding this comment.
Is this API necessary? the user can get the same result by adding a component directly to children.
As a side note, if you render all DOM nodes in a test, the snapshot becomes very large. |
|
Hi @Deep-Codes, we really appreciate your contribution!! See also: #262. |
Checklist
Change information
New Footer Component Added.
Covers all Types from link
Added Docs and Tests
Have followed Compound Component Pattern since it was used in Other Components too.
Needs more Suggestion on what Props should be passed.
Demo:
Attempts to Close #502