-
Notifications
You must be signed in to change notification settings - Fork 0
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
Chrome extension #1
base: dev
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for gleaming-sunshine-5cea24 failed.
|
✅ Deploy Preview for vaspacx-chrome-ext ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
The live preview hosted on Netlify is working well code structure looks good and easy to understand
- Awesome use of barrel exports and imports for importing multiple components and styles in a single line. Still at some more places can use it do check the codebase once to make sure of it.
- Remove commented useEffect based code and debuggers like console logs for checking the output.
- Good use of Animation libraries while developing landing page of the project👏🏻💯. Styled components reduced the abstraction while writing down components completely minimizing the use of CSS selectors
- Got to see a new approach of using redux toolkit along with managing local storage based variables with useEffect hook.
Awesome work on the extension Vashnavi. Got to learn & of course ask a lot🙂💯🚀
import { getDataActions } from "../store/extData"; | ||
|
||
const MainPage = () => { | ||
const dateValue = useSelector((state) => state.extData.dateStore.date); |
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.
Can write it altogether using object destructing as
const {dateStore:{date,month,year},flag} = useSelector((state)=>state.extData)
DetailContainer, | ||
UpperBox, | ||
} from "../components"; | ||
import SpecialCountdown from "../components/CountDown"; |
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.
Can use barrel exports for components from L11-L19.
city !== null | ||
? `q=${city}` | ||
: `lat=${coordinates.latitude}&lon=${coordinates.longitude}` | ||
}&appid=021cbeb6524e6c10b8d5ac497d7e73a3` |
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.
Can store this private data in an env variable otherwise the weather component might not work properly due to misuse of secret API key.
@@ -1,8 +1,15 @@ | |||
import { useSelector } from "react-redux"; | |||
import { BgWrapper, GlobalStyle } from "./components"; | |||
import MainPage from "./page/MainPage"; |
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.
Can use barrel exports here too.
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.
The code looks good to merge
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.
Nice work vashnavi.
Looks good to merge
live url:-https://vaspacx-chrome-ext.netlify.app/