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

feat: tagging of layers #144

Merged
merged 40 commits into from
Dec 17, 2021
Merged

feat: tagging of layers #144

merged 40 commits into from
Dec 17, 2021

Conversation

HideBa
Copy link
Member

@HideBa HideBa commented Dec 14, 2021

Overview

The developed tag system excludes the tag widget.
Users can create, delete, attach and detach tags.

What I've done

  • Developed
    • Tag component
    • Tag group component
    • Auto complete component
    • Scene tag pane component
    • Layer tag pane component
  • Changed
    • Select component

What I haven't done

  • Tag widget

Screenshot

スクリーンショット 2021-12-15 20 38 30

スクリーンショット 2021-12-15 20 38 11

スクリーンショット 2021-12-15 20 37 59

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.

@netlify
Copy link

netlify bot commented Dec 14, 2021

✔️ 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
Copy link

codecov bot commented Dec 16, 2021

Codecov Report

Merging #144 (f37b314) into main (dec1dd1) will increase coverage by 1.25%.
The diff coverage is 54.42%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
src/components/atoms/Flex/index.tsx 100.00% <ø> (ø)
src/gql/graphql-client-api.tsx 35.78% <29.41%> (-0.63%) ⬇️
src/components/atoms/Select/core.tsx 52.63% <52.63%> (ø)
src/components/atoms/AutoComplete/index.tsx 72.00% <72.00%> (ø)
src/components/atoms/SelectOption/index.tsx 80.00% <100.00%> (ø)
src/components/atoms/Tag/index.tsx 100.00% <100.00%> (ø)
src/components/atoms/Icon/index.tsx 64.70% <0.00%> (+5.88%) ⬆️

@HideBa HideBa marked this pull request as ready for review December 17, 2021 02:19
@HideBa HideBa changed the title feat: tag system feat: tag system without dataset Dec 17, 2021
@rot1024 rot1024 changed the title feat: tag system without dataset feat: tagging layers Dec 17, 2021
@rot1024 rot1024 changed the title feat: tagging layers feat: tagging of layers Dec 17, 2021
Copy link
Member

@rot1024 rot1024 left a 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

@lavalse
Copy link
Member

lavalse commented Dec 17, 2021

works good for me

Copy link
Member

@KaWaite KaWaite left a 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

@HideBa HideBa enabled auto-merge (squash) December 17, 2021 06:24
@HideBa HideBa merged commit 4d0a401 into main Dec 17, 2021
@HideBa HideBa deleted the tag-system branch December 17, 2021 06:28
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.

4 participants