-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
✔️ Deploy Preview for reearth-web ready! 🔨 Explore the source changes: f37b314 🔍 Inspect the deploy log: https://app.netlify.com/sites/reearth-web/deploys/61bc2cf19e363a000713ea0b 😎 Browse the preview: https://deploy-preview-144--reearth-web.netlify.app |
Codecov Report
@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 44.22% 45.48% +1.25%
==========================================
Files 56 60 +4
Lines 1135 1284 +149
Branches 179 205 +26
==========================================
+ Hits 502 584 +82
- Misses 580 641 +61
- Partials 53 59 +6
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor points (outside the scope of this PR)
- You can move atoms/SelectOption to both atoms/Select/SelectOption and atoms/AutoComplete/SelectOption (with copying)
- Move atoms/SelectBox to atoms/Select/SelectBox and export it from Select
- Refactor hooks
Co-authored-by: rot1024 <[email protected]>
Co-authored-by: rot1024 <[email protected]>
works good for me |
Co-authored-by: KaWaite <[email protected]>
Co-authored-by: KaWaite <[email protected]>
Co-authored-by: KaWaite <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I guess I don't have time to go through the code fully. But Inoue-san approved and you'll refactor next week so I'll go through more thoroughly then.
A couple things though from testing it out:
- There is a bug in the right panel's tabs. If you select Export or Tag, and then try to select something else, it'll go back to the layer tag
- the default tag group's name should be uppercased (default -> Default) (I also am not sure calling it Default is very nice, but we can discuss that later)
- There is ZERO indication of what each input does in the tag tab. It just says "default" and then has two separated inputs. If you can add placeholders like in @lavalse 's design ("Add a tag" and "Add a tag group")
Also, the PR is failing right now
Overview
The developed tag system excludes the tag widget.
Users can create, delete, attach and detach tags.
What I've done
What I haven't done
Screenshot
Which point I want you to review particularly
Memo
tag pane
component couldn't be a reusable component.Next year, I'll refactor the source code. If there isn't a critical problem plz approve.