-
Notifications
You must be signed in to change notification settings - Fork 87
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
changed the loading message and added a component and its style #193
base: main
Are you sure you want to change the base?
Conversation
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.
Changes looks good.
However, I doubt some files are in CRLF
line format instead of Linux's LF
line format. Please check.
@GMishx I use Linux instead of Windows |
Resolved the loader import issue as suggested by @soham4abc |
closes #185 |
There are some more issues that You need to fix... You need to change the .js files to .jsx and need to re import wherever they are used.. I cloned you code and fixed those issues in my local.. If you face any issues feel free to ask me |
@soham4abc I already did that. Check my latest push |
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.
After accepting my commit suggestions and pulling in You can do the following to get rid of the errors.
Change Loader.js
to Loader.jsx
Change messages.js
to messages.jsx
Use Global Search to change the import from:
import messages from "constants/messages";
to
import messages from "constants/messages.jsx";
There are total 18 occurrence of the above mentioned change
And add <Loader/>
instead of loading...
in index.jsx
line number 26. Do not forget to add the import for Loader there
Also one important thing, this repo follows a code formatting format. You can use Prettier code formatter (Available as VS code extension) and format the changed files so that it follows the formatting style of this repo.
Thank you!!
|
||
function Loader() { | ||
return ( | ||
<span class="loader"></span> |
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.
<span class="loader"></span> | |
<span className="loader"/> |
@DevEmmy are you still working on this issue? |
Is it working nice? If there are errors, you could let me know so I can fix it. |
there are some errors currently, I suggested some changes which should fix them. Do let me know if you have issues understanding them. :-) |
@DevEmmy , are there any updates on this PR? |
Description
Changing from "Loading..." text when a page is on refresh to a css animation.
Changes
In the Constant > messages.js file, I replaced the "loading..." text with a component which I titled "Loader.jsx"