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

Navigation Block - refactor edit function definition #22932

Closed
4 tasks
getdave opened this issue Jun 5, 2020 · 0 comments · Fixed by #23109
Closed
4 tasks

Navigation Block - refactor edit function definition #22932

getdave opened this issue Jun 5, 2020 · 0 comments · Fixed by #23109
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress [Type] Code Quality Issues or PRs that relate to code quality

Comments

@getdave
Copy link
Contributor

getdave commented Jun 5, 2020

...the edit function seems to be getting a bit big, I think it'd be nice if it were moved out and given a JSDoc description.

Originally posted by @talldan in https://github.com/_render_node/MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NDI0MjE0OTA5/pull_request_reviews/more_threads

See also:

I wouldn't say it's essential to do all refactoring in this PR, but it is, say ..., important to follow up quick enough :) so we don't forget it or even more, complicate it even further.

#18869 (comment)


The Problem

Essentially over time the edit function def has grown organically and it's now being unwieldy. Once #18869 is merged then this problem will become worse. Let's take a moment to refactor it to make it easier to work with and reason about.

Due to the complexity of the block and the rapid development currently underway let's try to break these out into tasks with 1 PR per task to keep it simple to review:

  • Extract the block Placeholder to its own component.
  • Extract rendered Block UI to its own component.
  • Create a utility for creating link blocks and then extract buildNavLinkBlocksFromMenuItems and buildNavLinkBlocksFromPages to utilise this.
  • Make handleCreate pure-er by providing arguments directly (eg: selectedCreateActionOption, hasPages...etc).

cc @talldan @draganescu

@getdave getdave added the [Block] Navigation Affects the Navigation Block label Jun 5, 2020
@draganescu draganescu added the [Type] Code Quality Issues or PRs that relate to code quality label Jun 5, 2020
@talldan talldan self-assigned this Jun 11, 2020
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants