add placeholders #337
add placeholders #337
Conversation
There was a problem hiding this comment.
@aquibyatoo Can you add documentation so that it is possible for me to test it out quickly? You can refer other component directories to get an idea on how to document it.
|
yeah by mistake. Its only shape there, fixed.
thanks
…On Thu, Jul 25, 2019 at 5:50 PM Rishi Kumar Chawda ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/placeholders/box/readMe.md
<#337 (comment)>
:
> @@ -0,0 +1,31 @@
+## Navbar
+
+The box placeholder component.
+
+### Properties
+
+| Name | Type | Default | Description |
+| :------- | :------- | :-------- | :----------------------------------- |
+| `square` | `circle` | | Shapes of the box default is circle. |
I can't see square prop used anywhere in the component. Is this by
mistake?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#337?email_source=notifications&email_token=AEX744X4NIDPXLVY67WPRBDQBGK7TA5CNFSM4IGOD6XKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7R4SOQ#pullrequestreview-266586426>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEX744Q4ZDRJ4SISWLNGO53QBGK7TANCNFSM4IGOD6XA>
.
|
|
Actually its because i have configured prettier already and that is why i
put it in .gitignore.
thanks
…On Thu, Jul 25, 2019 at 5:52 PM Rishi Kumar Chawda ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .gitignore
<#337 (comment)>
:
> \ No newline at end of file
+/components
+/.prettierrc.json
why was prettier used? We already have eslint set up and an eslintrc on
the root.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#337?email_source=notifications&email_token=AEX744Q4LNWYZGCDZBXOW73QBGLG7A5CNFSM4IGOD6XKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7R4Z3I#pullrequestreview-266587373>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEX744W7ZBPUX3SS56VQCATQBGLG7ANCNFSM4IGOD6XA>
.
|
| 100% { | ||
| width: 100%; | ||
| } | ||
| -o-animation-name: animateFocus; |
There was a problem hiding this comment.
Looks like this was due to prettier. Can you try removing prettier and lint files with eslint and see if it reverts these changes?
There was a problem hiding this comment.
Regarding the functionalities and current configuration of the components :
-
I can see there are two components with nearly same function :
ImageCardandCardPlaceholder. I couldn't spot any functional difference between them and they look almost identical to each other too. -
Why do we have two exports :
ParagraphPlaceHolderandLinePlaceholder- can it be done in a single component? -
ParagraphPlaceHolderseems to have box shadow. User might not want it in most of cases probably? -
The "shimmer" effect looks like it is not completely carried out till the other end of the placeholders.
-
In
CardPlaceholderthe image looks like it has shimmer faster than the lines ( probably because lines are longer than the width of the card ). Can this be tweaked so it looks balanced? -
The line widths in
ParagraphPlaceHolderhave a fixed value which increments as the lines proceed. Can this be randomised so that it looks more natural? -
In
CardPlaceholdercomponent, the line widths are hard-coded. Can this be changed too? Can we also use the existingCardcomponent instead of having a separate wrapper again? Since it has other options too like changing shadows, etc. Moreover, it would be then an easier content management from user perspective. Also, we won't have to ship a component like this altogether ( Can be put as a demo usingCardandLinePlaceholdercomponent.
incremental builds took a longer time that greatly effected developer experience, this commit improves the rebuild time and generates outputs almost instantaneously.
There was a problem hiding this comment.
Regarding the functionalities and current configuration of the components :
I can see there are two components with nearly same function : ImageCard and CardPlaceholder. I couldn't spot any functional difference between them and they look almost identical to each other too.
Why do we have two exports : ParagraphPlaceHolder and LinePlaceholder - can it be done in a single component?
ParagraphPlaceHolder seems to have box shadow. User might not want it in most of cases probably?
The "shimmer" effect looks like it is not completely carried out till the other end of the placeholders.
In CardPlaceholder the image looks like it has shimmer faster than the lines ( probably because lines are longer than the width of the card ). Can this be tweaked so it looks balanced?
The line widths in ParagraphPlaceHolder have a fixed value which increments as the lines proceed. Can this be randomised so that it looks more natural?
In CardPlaceholder component, the line widths are hard-coded. Can this be changed too? Can we also use the existing Card component instead of having a separate wrapper again? Since it has other options too like changing shadows, etc. Moreover, it would be then an easier content management from user perspective. Also, we won't have to ship a component like this altogether ( Can be put as a demo using Card and LinePlaceholder component.
@aquibyatoo Are all the changes present in this branch?
|
Closing this PR. |
Status :
In Development
Description :
Add placeholders for loading state for app. There are Box, Line, Image Card, Card and Paragraph placeholders.
Related Issues :
#169
Todos