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

Initial ideas for overlay refactor #12

Closed
wants to merge 3 commits into from
Closed

Conversation

richagadgil
Copy link
Member

@richagadgil richagadgil commented Feb 28, 2021

Ideas for Refactor:

  • Each overlay has only one interaction type
  • Create system of inherited types

Addresses Issues #8 and #11

@Waidhoferj
Copy link
Member

We may want to consider a MultiActionOveraly for UI elements that may include multiple actions (e.g. a teacher's office modal might have a link to their website and a teleport button that takes you to their classroom).

@Waidhoferj
Copy link
Member

Also, I noticed Jason and Kalen working on a setup that mapped the JSON directly to React components. This could be a more streamlined way to present the overlays on the screen. Something like this:

overlays.map(overlay => {
switch(overlay.type)
    case "path":
           return <PathOverlay {...overlay}/>
    case "teleport":
           return <TeleportOverlay {...overlay}/>
   ...........
}

@kgoo124
Copy link
Member

kgoo124 commented Mar 1, 2021

@Waidhoferj can we make the assumption that every overlay has a title and description? And then render an OverlayInteractions component which is an array of buttons each having their own action (showing more info, traveling, etc.). Then, we would render an Overlay every time and render any interactions available within it.

Names are arbitrary, I just added them to provide context.

@Waidhoferj
Copy link
Member

@kgoo124 I really like that idea.

@kgoo124
Copy link
Member

kgoo124 commented Mar 7, 2021

Decided to go with a different design pattern, new PR is #14

@kgoo124 kgoo124 closed this Mar 7, 2021
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