Skip to content
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: project init structure with basic layout #4

Merged
merged 12 commits into from
Sep 28, 2024

Conversation

rishavvajpayee
Copy link
Contributor

changes:

  • Set up the Next.js project structure.
  • Created basic terminal and search components.
  • Ensured responsive layout with consistent navbar and footer heights.

@rishavvajpayee
Copy link
Contributor Author

@lucifercr07 hey please Let me know if NextJS / project structure changes are required.

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishavvajpayee can you also update README.md with instructions around how to start and develop. You can refer to DiceDB doc to maintain parity.

@rishavvajpayee
Copy link
Contributor Author

@lucifercr07 updated the ReadME file with the required instructions

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check getting error in build

   Linting and checking validity of types  ..Failed to compile.

./app/components/PlaygroundTerminal.tsx:6:46
Type error: Binding element 'output' implicitly has an 'any' type.

  4 | import { Dice1 } from 'lucide-react';
  5 |
> 6 | export default function PlaygroundTerminal({ output, command, setCommand, handleCommand }) {
    |                                              ^
  7 |   const terminalRef = useRef<HTMLDivElement>(null);
  8 |
  9 |   useEffect(() => {

@rishavvajpayee
Copy link
Contributor Author

hey @lucifercr07 added the missing type checks. build is working now. Sorry didn't checked the run build command before pushing the PR.

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rishavvajpayee for contribution, we can get on a call if you want and iron out few changes suggested

/>
</div>
</div>
<div className="space-y-2">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially we don't need to set values, it can be empty.
image

))}
</div>
<div className="flex items-center bg-gray-700 rounded px-2 mb-4">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishavvajpayee shall we try to maintain similarity with redis-cli?
For reference:
image

<div className="flex items-center bg-gray-700 rounded px-2 mb-4">
<Dice1 className="w-4 h-4 text-green-500 mr-2" />
<span className="text-green-500">dice&gt;</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just keep white text for now on blackbackground.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey i went ahead and tried to copy the UI of dicedb.io we can change the UI later on.

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor changes we'll require can be taken up in following PRs.
Thanks @rishavvajpayee for contributing.

@lucifercr07 lucifercr07 merged commit 02d9156 into DiceDB:master Sep 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants