-
Notifications
You must be signed in to change notification settings - Fork 621
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
Page builder blocks v2 UI list page blocks #2496
Page builder blocks v2 UI list page blocks #2496
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.
Just a couple of comments.
import React, { useCallback, useMemo, useState } from "react"; | ||
import { i18n } from "@webiny/app/i18n"; | ||
import { useRouter } from "@webiny/react-router"; | ||
import { useQuery } from "@apollo/react-hooks"; |
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 you clarify please, what is this view? Can you provide a screenshot?
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.
Was about to merget this, but I noticed one thing. @neatbyte-vnobis , should this be "Search categories" here? Maybe "Search blocks" ? |
@adrians5j Design (https://miro.com/app/board/uXjVO1XGqwg=/): |
We are searching for blocks, that’s correct. |
@SvenAlHamad @adrians5j
I will have to rewrite quite a few things to do it: |
@neatbyte-vnobis In all of our views, these search inputs are doing a search over the entity the user's currently looking at. For example, on the pages view, pages are being searched. On the page categories view, page categories are being searched. So, if we were to follow the above examples and wanted to stay consistent, typing into the search field would do a search against existing page blocks. But! Now when I look at it, I can see that this view IS a bit different than the other ones we have. Instead of just showing blocks in the list, we're showing block categories. Once clicked, the blocks are shown on the right. And now I do think we need to think about this. I do agree that this should search for blocks because the user will most probably want to do that (instead of searching for block categories). But I can also see that, in the provided designs, the label said So.... what I'd do here now is....... @SvenAlHamad - please confirm the direction into which we want to move. Do we want to leave it as-is (and save potentially some time), or do we want to revisit this now, and make the search bar do a search over blocks? Hopefully, I didn't miss anything (like some docs), but yeah, let's first sync on what exactly we want to have here. |
Ok, let's leave it so it's searching blocks if that's what's currently implemented. Once we start using it a bit I think we'll have then one milestone addressing the initial feedback and some additional polishes. |
Actually, the way it's currently implemented is that it's searching for block categories, not blocks. I'd still leave it as is, so we can address it then in the MS you mentioned. |
Sorry, I miss typed in my previous comment. I DO understand it's searching "block categories" and I'm happy to leave it like that for now. |
@neatbyte-vnobis feel free to merge this one. Thank you! |
Changes
Add Page Blocks page to Page Builder UI app
How Has This Been Tested?
Manual testing
Documentation
None