Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

feat: multi widgets #99

Merged
merged 20 commits into from
Oct 21, 2021
Merged

feat: multi widgets #99

merged 20 commits into from
Oct 21, 2021

Conversation

KaWaite
Copy link
Member

@KaWaite KaWaite commented Oct 7, 2021

Overview

Up until now, widgets could only have one instance. Also, we were unable to remove a widget (and its data) completely, only disable. This brings both.

What I've done

  • Added removeWidget mutation for widget deletion
  • Updated the UI
    • Moved enabling toggle to the left panel (shows on hover or selection)
    • Add delete button
    • Add dropdown of available widgets (from installed plugins)

The dropdown showing installed widgets will grey out widgets that are single-use and already have a single instance.
Trying to delete a selected widget will show a confirmation modal before deleting.

What I haven't done

I haven't fixed an issue with the LayerTreeView components where the whole tree will be re-rendered on ANY changes made within (this includes local state). This affects the toggle's transition so it doesn't smoothly move, as well as the widget dropdown is contained within the left panel (where it should come out slightly over Cesium. Also affects the left panel's help balloons) and it will be slightly clipped the first time opening the dropdown.

How I tested

  • Add/Delete widgets
  • Enable/disable widgets
  • Try to add a single-use widget(Storytelling) a second time (should be greyed out)

Screenshot

Screen Shot 2021-10-11 at 13 11 14

Screen Shot 2021-10-11 at 13 11 21

Screen Shot 2021-10-11 at 13 11 29

Which point I want you to review particularly

Memo

@netlify
Copy link

netlify bot commented Oct 7, 2021

✔️ Deploy Preview for reearth-web ready!

🔨 Explore the source changes: 37f1e70

🔍 Inspect the deploy log: https://app.netlify.com/sites/reearth-web/deploys/6170e089b25f4d0008362102

😎 Browse the preview: https://deploy-preview-99--reearth-web.netlify.app

@codecov
Copy link

codecov bot commented Oct 8, 2021

Codecov Report

Merging #99 (37f1e70) into main (25a2171) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #99   +/-   ##
=======================================
  Coverage   53.53%   53.53%           
=======================================
  Files          48       48           
  Lines         891      891           
  Branches      119      119           
=======================================
  Hits          477      477           
  Misses        364      364           
  Partials       50       50           
Impacted Files Coverage Δ
src/gql/graphql-client-api.tsx 36.87% <ø> (ø)
src/theme/darkTheme.ts 100.00% <ø> (ø)
src/theme/lightheme.ts 100.00% <ø> (ø)
src/theme/theme.ts 100.00% <ø> (ø)

@KaWaite KaWaite self-assigned this Oct 11, 2021
@KaWaite KaWaite marked this pull request as ready for review October 11, 2021 04:12
@KaWaite KaWaite requested review from rot1024, HideBa and issmail-basel and removed request for rot1024 October 11, 2021 04:12
Copy link
Member

@HideBa HideBa left a comment

Choose a reason for hiding this comment

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

I've left a comment which is about the entire review. Thereby I won't review detail until we make a decision. Plz, take a look at my comment.

@rot1024
Copy link
Member

rot1024 commented Oct 12, 2021

This PR should be reviewed by me at least once. Please don't merge this PR until then.

@KaWaite KaWaite requested a review from HideBa October 18, 2021 06:52
HideBa
HideBa previously approved these changes Oct 20, 2021
Copy link
Member

@HideBa HideBa left a comment

Choose a reason for hiding this comment

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

LGTM!

@KaWaite KaWaite merged commit bea1a32 into main Oct 21, 2021
@KaWaite KaWaite deleted the feat/multi-widgets branch October 21, 2021 03:56
keiya01 pushed a commit that referenced this pull request Apr 25, 2023
* rename widget system and plugin system

* refactor tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants