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

src/components/atoms/Select/index.tsx Outdated Show resolved Hide resolved
translations/ja.yml Outdated Show resolved Hide resolved
translations/ja.yml Outdated Show resolved Hide resolved
translations/ja.yml Outdated Show resolved Hide resolved
HideBa and others added 2 commits December 16, 2021 23:43
Co-authored-by: rot1024 <aayhrot@gmail.com>
Co-authored-by: rot1024 <aayhrot@gmail.com>
@lavalse
Copy link
Member

lavalse commented Dec 17, 2021

works good for me

Co-authored-by: KaWaite <34051327+KaWaite@users.noreply.github.com>
Co-authored-by: KaWaite <34051327+KaWaite@users.noreply.github.com>
HideBa and others added 2 commits December 17, 2021 00:28
Co-authored-by: KaWaite <34051327+KaWaite@users.noreply.github.com>
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
4 participants