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

Move and refactor Inserter component as reused in header tools #403

Merged
merged 4 commits into from
Apr 11, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 11, 2017

This pull request seeks to apply a few refactorings to the <Inserter /> component:

  • Move from editor/inserter to editor/components/inserter
  • Change entry point from components/inserter/button to components/inserter
    • Should enforce a general preference toward index.js as entry point of any component. If we want menu to be a separate independent component, we should move it to its own directory to reflect as such, but currently we don't have a need for this.
  • Render <Inserter /> in Header tools

Testing Instructions:

Verify that an inserter button is shown both in the header and at the bottom of post content and can be toggled to display a menu overlay.

Future Tasks:

  • Header inserter overlay position should open downward, not upward
  • To the first point, we may want to consider extracting overlay/tooltip logic to a separate component (perhaps with logic detecting adapting to display in opposite direction when desired opening direction exceeds viewport bounds)

@aduth aduth added the Framework Issues related to broader framework topics, especially as it relates to javascript label Apr 11, 2017
@aduth aduth requested a review from youknowriad April 11, 2017 15:26
@youknowriad
Copy link
Contributor

If we want menu to be a separate independent component, we should move it to its own directory to reflect as such, but currently we don't have a need for this.

Actually, this was the intention, since the inserter will be used without the button (typing /)

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@aduth aduth merged commit b864b6b into master Apr 11, 2017
@aduth aduth deleted the move/inserter branch April 11, 2017 15:55
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jul 14, 2022
@Mamaduka Mamaduka removed their assignment Jul 14, 2022
@Mamaduka Mamaduka removed their assignment Oct 19, 2022
@Mamaduka Mamaduka removed their assignment Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants