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

Adding new design+new download functionality #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

saiphanindra1010
Copy link

Added new features and improvements:

  • Implemented a download button to download a .txt file from CLI code.
  • Made the UI slightly responsive.
  • Added a navbar with dummy links and a dark mode icon.

Before:
Screenshot 2024-10-01 210749

After:
Screenshot 2024-10-01 211018

@saiphanindra1010
Copy link
Author

@lucifercr07 can you review

<div className="border rounded-lg p-4">
<h2 className="text-xl font-semibold mb-4">Recent Commands</h2>
<ul className="space-y-2 border">
{['SELECT * FROM users', 'INSERT INTO products', 'UPDATE orders SET status = "shipped"'].map((cmd, index) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Why's this required?

Copy link
Author

Choose a reason for hiding this comment

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

We can use the browser session to populate recently run commands. I've just kept a placeholder array for now. Do you think this feature is worth implementing?

Copy link
Contributor

Choose a reason for hiding this comment

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

For now I'm not sure, let's create an issue out of it and we can pick this up later after we go live.

import Cli from "@/components/CLI/CLI";
import SearchBox from "@/components/Search/SearchBox";
import { Dice1, Dice3, Dice5 } from "lucide-react";
import { Dice1, Dice3, Dice5, Clock, Command, Sun, Moon, Github, Download } from "lucide-react";
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 remove the download functionality for now, please open an issue for this we can pick up later.



const NavBar: React.FC<{ isDarkMode: boolean; toggleDarkMode: () => void }> = ({ isDarkMode, toggleDarkMode }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to keep things in parity with our website dicedb.io, please open an issue for DarkMode we can take this up later.

@lucifercr07
Copy link
Contributor

@saiphanindra1010 thanks a lot for your contribution. I'd suggest let's just limit the scope of this PR to Cleanup and Commands left for now.
Please align docs, blog etc. to be in parity with dicedb.io.

Also please contribute test cases for components, may be as a separate PR, we're eagerly looking for that.

@saiphanindra1010
Copy link
Author

saiphanindra1010 commented Oct 1, 2024

@lucifercr07 do I close pr I see one more pr which is fixing ui
#14

@lucifercr07
Copy link
Contributor

@saiphanindra1010 we can keep your header changes and cleanup timer, commands left changes. Rest we can create an issue and fix.

@saiphanindra1010
Copy link
Author

@lucifercr07 done removed download and hardcoded data

image

@lucifercr07
Copy link
Contributor

@saiphanindra1010 recent commands is also not required for now I believe. We can add these functionalities later. Also as mentioned in above comment please align docs, blog etc. in header to be in parity with dicedb.io website.

@saiphanindra1010
Copy link
Author

@lucifercr07 All the links are pointing to the correct links and reverted the commends

@lucifercr07
Copy link
Contributor

image

We need to keep title as DiceDB Playground itself and align other blogs etc. in same header.

Try to replicate like below if possible
image

@saiphanindra1010
Copy link
Author

@lucifercr07 in lucid-react brand icons are deprecated do I use a different package?

@saiphanindra1010
Copy link
Author

this is how it looks without logos
image

cc @lucifercr07

@lucifercr07
Copy link
Contributor

@saiphanindra1010 please fix the merge conflicts.

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