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

Load Entity into Block functionality #480

Merged
merged 51 commits into from
Apr 15, 2022
Merged

Conversation

teenoh
Copy link
Contributor

@teenoh teenoh commented Apr 11, 2022

🌟 What is the purpose of this PR?

This PR adds code to support loading an entity into a block. It also updates BlockContextMenu to match the new designs. Right now it only supports block entities (entities created via a block) since they match the block's schema. Eventually it will be able to support other entities.

⚠️ Known issues

  • There's a bug where the block gets reset to its initial state after it's entity has been changed. task. There's a temporary workaround for the time being. Once this has been fixed, the feature flag will be removed.
  • BlockContextMenu isn't pixel perfect. There is a layout shift when the text field in the addEntity submenu has a loading state. This will be addressed when in Styles for Selection Menu
  • BlockContextMenu has some keyboard navigation issues. This will be addressed in nested menu task
  • There are some typing issues which I have temporarily ignored just to get this in. Will address when working on task
  • createLocalBlock doesn't properly handle text blocks and that affects loading entities in text blocks . This will be addressed in Fix bug in loading entities in text blocks which will be worked on after Add block config panel. Add block schema to block context #490 has been merged in, since it provides a synchronous way to access the blocks componentSchema and that is needed to determine if a block is a text block

🚫 Blocked by

  • ...

🐾 Next steps

🔍 What does this change?

  • Adds a useAccountEntities hook for fetching entities based on block's componentId
  • Adds LoadEntityMenuContent and BlockListMenuContent which are submenus for loading an entity and swapping blocks
  • Updates BlockContextMenu to match new designs
  • Adds a new variant xs to the TextField component and fixes a console error that happens as a result of FormHelperText
  • Adds shortcut to handle blocks/entities search. When BlockContextMenu is open, clicking on @ opens the load entity submenu while / opens blocklist sub menu
  • Adds an entity store action => updateBlockEntityProperties to handle updating a block's entity in draft entity store.
  • Adds a createLocalBlock method in ProsemirrorSchemaManager for creating blocks whose info have already been fetched.
  • Adds updateBlockData to handle updating a block's data and prosemirror tree
  • The UI for loading entities in block is hidden behind a feature flag for the time being. It will be completely accessible once the bug has been fixed.

📜 Does this require a change to the docs?

  • No

🔗 Related links

🛡 What tests cover this?

  • Existing playwright tests

❓ How to test this?

  1. Start the app
  2. Add hash-load-entity-ui to local-storage
  3. Confirm BlockContextMenu works as before (test loading a block, changing a block)
  4. Create multiple blocks with data in a page in order to generate a list of block entities that can be switched through
  5. Try changing the entity of a block via BlockContextMenu

📹 Demo

Screen.Recording.2022-04-12.at.08.54.26.mov
Screen.Recording.2022-04-12.at.09.00.40.mov

@github-actions github-actions bot added area/apps > hash* Affects HASH (a `hash-*` app) frontend labels Apr 11, 2022
@lgtm-com
Copy link

lgtm-com bot commented Apr 11, 2022

This pull request introduces 9 alerts when merging d9513cf into f94f332 - view on LGTM.com

new alerts:

  • 8 for Unused variable, import, function or class
  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Apr 11, 2022

This pull request introduces 1 alert when merging 48614a7 into e6f3ba8 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2022

This pull request introduces 1 alert when merging c867f5c into 3063e76 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@vercel vercel bot temporarily deployed to Preview – hash April 14, 2022 16:14 Inactive
@vercel vercel bot temporarily deployed to Preview – hashdev April 14, 2022 16:14 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2022

This pull request introduces 1 alert when merging 6ce3d8e into 3063e76 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@teenoh teenoh requested review from CiaranMn and nathggns April 14, 2022 16:23
@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2022

This pull request introduces 1 alert when merging a6d09c6 into 3063e76 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@vercel vercel bot temporarily deployed to Preview – hashdev April 15, 2022 09:50 Inactive
@vercel vercel bot temporarily deployed to Preview – hash April 15, 2022 09:50 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 1 alert when merging 22f7792 into 3063e76 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 1 alert when merging 99f1d29 into 3063e76 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@teenoh teenoh merged commit 964109f into main Apr 15, 2022
@teenoh teenoh deleted the vu/instantiate-block-with-entity branch April 15, 2022 12:35
@vilkinsons vilkinsons added type/eng > frontend Owned by the @frontend team area/tests > playwright New or updated Playwright tests and removed area: frontend labels Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apps > hash* Affects HASH (a `hash-*` app) area/tests > playwright New or updated Playwright tests type/eng > frontend Owned by the @frontend team
Development

Successfully merging this pull request may close these issues.

4 participants