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

Layers page #31

Merged
merged 3 commits into from
Nov 21, 2023
Merged

Layers page #31

merged 3 commits into from
Nov 21, 2023

Conversation

maithili232
Copy link
Contributor

@maithili232 maithili232 commented Nov 8, 2023

Added layers page to the website. Followed the same pattern of model page.

Copy link
Member

@rbharath rbharath left a comment

Choose a reason for hiding this comment

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

@Yukino2002 Can you review this PR?

@maithili232 Can you put up screenshots of what this looks like?

@@ -227,3 +228,49 @@ footer {
@apply pb-4;
}
}

/* layers page */
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are using the exact same styling, you can just reuse the classes. No need to redeclare with a different class name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -0,0 +1,53 @@
import Image from "next/image";
Copy link
Contributor

Choose a reason for hiding this comment

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

This component is also exactly the same as Models/FilterButton.jsx. Please reuse the component, maybe move it to a directory named Common and use it in both models and layers page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

const Layers = () => {

const [filteredLayers, setFilteredLayers] = useState(layers);
const [models, setModels] = useState([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure what models exactly are in the layers page, is the naming intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Models that use the layers so layers can be filtered with their respective models.

@Yukino2002
Copy link
Contributor

Please add the layers page as a card in the landing page or button in the navigation bar, so that it is accessible to users.

@Yukino2002
Copy link
Contributor

Yukino2002 commented Nov 8, 2023

@maithili232 Added a few comments, will check on local once the comments have been addressed.

@@ -0,0 +1,59 @@
import Image from "next/image";
import Link from "next/link";
Copy link
Contributor

Choose a reason for hiding this comment

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

This component is also very similar. I think some part has been omitted out, but I think can be reused.

@maithili232
Copy link
Contributor Author

@Yukino2002 Can you review this PR?

@maithili232 Can you put up screenshots of what this looks like?

Sharing few screenshots :
image
image
image

@maithili232
Copy link
Contributor Author

maithili232 commented Nov 10, 2023

Please add the layers page as a card in the landing page or button in the navigation bar, so that it is accessible to users.

I have added the layers on the index page like this :
image
I have used the datasets logo for it. Let me know if its alright or not.

@rbharath rbharath merged commit 42c6ca2 into deepchem:main Nov 21, 2023
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.

3 participants