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

chore: add issue template for new components #152

Merged
merged 8 commits into from
Oct 27, 2021

Conversation

landondurnan
Copy link
Contributor

@landondurnan landondurnan commented Sep 30, 2021

Describe the problem this PR addresses

As more engineers are contributing new components to Maker I thought it would be helpful for communication and collaboration to ensure we're getting more information about a component before engineering work starts.

Building a new component from scratch requires more information and communication than simply adding a feature to an existing component. For example, a link to Figma with the design documentation ensures that we're all referencing the correct design files when discussing those requirements (there are internal examples of this not happening and causing confusion). Each question in this new issue template outline what's required as a minimum along with several suggested criteria for consideration of adding a new component to the system.

Describe the changes in this PR

A new issue template that provides a form output specifically for adding new components to Maker. Can't take a screenshot until it exists.

Other information

The existing new feature template is sufficient for general additions to the library, but it's easy to overlook many aspects that help us collaborate and review considerations specifically for new components.

@github-actions
Copy link

@github-actions
Copy link

github-actions bot commented Sep 30, 2021

📊 Package size report   No changes

File Before After
Total (Includes all files) 1.0 MB 1.0 MB
Tarball size 187.7 kB -0%↓187.7 kB
Unchanged files
File Size
components/ActionBar/index.js 46 B
components/ActionBar/script.js 13.1 kB
components/ActionBar/styles.css 5.9 kB
components/Blade/index.js 46 B
components/Blade/script.js 6.3 kB
components/Blade/styles.css 756 B
components/Button/index.js 46 B
components/Button/script.js 7.4 kB
components/Button/styles.css 5.0 kB
components/Calendar/index.js 46 B
components/Calendar/script.js 7.5 kB
components/Calendar/styles.css 2.5 kB
components/Card/index.js 46 B
components/Card/script.js 1.7 kB
components/Card/styles.css 154 B
components/Checkbox/index.js 46 B
components/Checkbox/script.js 3.8 kB
components/Checkbox/styles.css 1.7 kB
components/Choice/index.js 46 B
components/Choice/script.js 4.3 kB
components/Choice/styles.css 1.3 kB
components/Container/index.js 46 B
components/Container/script.js 4.3 kB
components/Container/styles.css 1.0 kB
components/Dialog/index.js 46 B
components/Dialog/script.js 6.3 kB
components/Dialog/styles.css 1.0 kB
components/Divider/index.js 46 B
components/Divider/script.js 1.7 kB
components/Divider/styles.css 148 B
components/Heading/index.js 46 B
components/Heading/script.js 2.6 kB
components/Heading/styles.css 3.1 kB
components/Image/index.js 46 B
components/Image/script.js 3.4 kB
components/Image/styles.css 254 B
components/ImageUploader/index.js 46 B
components/ImageUploader/script.js 11.9 kB
components/ImageUploader/styles.css 2.5 kB
components/Input/index.js 46 B
components/Input/script.js 4.3 kB
components/Input/styles.css 3.1 kB
components/Loading/index.js 46 B
components/Loading/script.js 2.3 kB
components/Loading/styles.css 1.2 kB
components/Modal/index.js 46 B
components/Modal/script.js 7.8 kB
components/Modal/styles.css 969 B
components/Notice/index.js 46 B
components/Notice/script.js 4.3 kB
components/Notice/styles.css 1.0 kB
components/ProgressBar/index.js 46 B
components/ProgressBar/script.js 3.0 kB
components/ProgressBar/styles.css 1.1 kB
components/Radio/index.js 46 B
components/Radio/script.js 3.5 kB
components/Radio/styles.css 1.6 kB
components/SegmentedControl/index.js 46 B
components/SegmentedControl/script.js 3.1 kB
components/SegmentedControl/styles.css 813 B
components/Select/index.js 46 B
components/Select/script.js 5.1 kB
components/Select/styles.css 2.5 kB
components/Skeleton/index.js 46 B
components/Skeleton/script.js 4.4 kB
components/Skeleton/styles.css 817 B
components/StarRating/index.js 46 B
components/StarRating/script.js 6.0 kB
components/StarRating/styles.css 322 B
components/Stepper/index.js 46 B
components/Stepper/script.js 3.8 kB
components/Stepper/styles.css 384 B
components/Text/index.js 46 B
components/Text/script.js 2.1 kB
components/Text/styles.css 1.0 kB
components/Textarea/index.js 46 B
components/Textarea/script.js 3.7 kB
components/Textarea/styles.css 2.2 kB
components/Theme/index.js 25 B
components/Theme/script.js 2.3 kB
components/Toggle/index.js 46 B
components/Toggle/script.js 3.8 kB
components/Toggle/styles.css 3.6 kB
components/TransitionFadeIn/index.js 25 B
components/TransitionFadeIn/script.js 2.3 kB
components/TransitionResize/index.js 25 B
components/TransitionResize/script.js 3.7 kB
components/TransitionSpringLeft/index.js 25 B
components/TransitionSpringLeft/script.js 2.3 kB
components/TransitionSpringUp/index.js 25 B
components/TransitionSpringUp/script.js 2.3 kB
LICENSE 552 B
package.json 4.9 kB
README.md 327 B
utils/assert.js 767 B
utils/BlockFormControlLayout/index.js 46 B
utils/BlockFormControlLayout/script.js 1.8 kB
utils/BlockFormControlLayout/styles.css 333 B
utils/get-contrast.js 1.2 kB
utils/InlineFormControlLayout/index.js 46 B
utils/InlineFormControlLayout/script.js 2.5 kB
utils/InlineFormControlLayout/styles.css 355 B
utils/Transition/index.js 25 B
utils/Transition/script.js 2.5 kB
utils/TransitionResponsive/index.js 25 B
utils/TransitionResponsive/script.js 2.3 kB
utils/transitions.js 3.9 kB
Hidden files
File Before After
components/ActionBar/script.js.map 51.4 kB 51.4 kB
components/ActionBar/styles.css.map 16.6 kB 16.6 kB
components/Blade/script.js.map 23.4 kB 23.4 kB
components/Blade/styles.css.map 3.9 kB 3.9 kB
components/Button/script.js.map 31.0 kB 31.0 kB
components/Button/styles.css.map 14.0 kB 14.0 kB
components/Calendar/script.js.map 29.0 kB 29.0 kB
components/Calendar/styles.css.map 10.4 kB 10.4 kB
components/Card/script.js.map 8.8 kB 8.8 kB
components/Card/styles.css.map 668 B 668 B
components/Checkbox/script.js.map 18.4 kB 18.4 kB
components/Checkbox/styles.css.map 3.9 kB 3.9 kB
components/Choice/script.js.map 19.0 kB 19.0 kB
components/Choice/styles.css.map 5.7 kB 5.7 kB
components/Container/script.js.map 17.0 kB 17.0 kB
components/Container/styles.css.map 4.6 kB 4.6 kB
components/Dialog/script.js.map 23.6 kB 23.6 kB
components/Dialog/styles.css.map 4.4 kB 4.4 kB
components/Divider/script.js.map 8.8 kB 8.8 kB
components/Divider/styles.css.map 704 B 704 B
components/Heading/script.js.map 13.5 kB 13.5 kB
components/Heading/styles.css.map 5.5 kB 5.5 kB
components/Image/script.js.map 14.1 kB 14.1 kB
components/Image/styles.css.map 2.9 kB 2.9 kB
components/ImageUploader/script.js.map 46.3 kB 46.3 kB
components/ImageUploader/styles.css.map 20.5 kB 20.5 kB
components/Input/script.js.map 21.1 kB 21.1 kB
components/Input/styles.css.map 6.2 kB 6.2 kB
components/Loading/script.js.map 11.1 kB 11.1 kB
components/Loading/styles.css.map 2.3 kB 2.3 kB
components/Modal/script.js.map 28.7 kB 28.7 kB
components/Modal/styles.css.map 7.5 kB 7.5 kB
components/Notice/script.js.map 17.4 kB 17.4 kB
components/Notice/styles.css.map 3.8 kB 3.8 kB
components/ProgressBar/script.js.map 13.3 kB 13.3 kB
components/ProgressBar/styles.css.map 2.6 kB 2.6 kB
components/Radio/script.js.map 17.2 kB 17.2 kB
components/Radio/styles.css.map 3.7 kB 3.7 kB
components/SegmentedControl/script.js.map 14.1 kB 14.1 kB
components/SegmentedControl/styles.css.map 3.3 kB 3.3 kB
components/Select/script.js.map 23.7 kB 23.7 kB
components/Select/styles.css.map 6.5 kB 6.5 kB
components/Skeleton/script.js.map 17.7 kB 17.7 kB
components/Skeleton/styles.css.map 2.9 kB 2.9 kB
components/StarRating/script.js.map 22.4 kB 22.4 kB
components/StarRating/styles.css.map 6.3 kB 6.3 kB
components/Stepper/script.js.map 16.1 kB 16.1 kB
components/Stepper/styles.css.map 3.3 kB 3.3 kB
components/Text/script.js.map 10.4 kB 10.4 kB
components/Text/styles.css.map 2.7 kB 2.7 kB
components/Textarea/script.js.map 18.5 kB 18.5 kB
components/Textarea/styles.css.map 4.8 kB 4.8 kB
components/Theme/script.js.map 10.8 kB 10.8 kB
components/Toggle/script.js.map 19.6 kB 19.6 kB
components/Toggle/styles.css.map 5.4 kB 5.4 kB
components/TransitionFadeIn/script.js.map 10.5 kB 10.5 kB
components/TransitionResize/script.js.map 14.5 kB 14.5 kB
components/TransitionSpringLeft/script.js.map 10.6 kB 10.6 kB
components/TransitionSpringUp/script.js.map 10.5 kB 10.5 kB
utils/assert.js.map 2.4 kB 2.4 kB
utils/BlockFormControlLayout/script.js.map 8.5 kB 8.5 kB
utils/BlockFormControlLayout/styles.css.map 902 B 902 B
utils/get-contrast.js.map 5.0 kB 5.0 kB
utils/InlineFormControlLayout/script.js.map 11.9 kB 11.9 kB
utils/InlineFormControlLayout/styles.css.map 1.2 kB 1.2 kB
utils/Transition/script.js.map 10.8 kB 10.8 kB
utils/TransitionResponsive/script.js.map 10.3 kB 10.3 kB
utils/transitions.js.map 14.1 kB 14.1 kB

🤖 This report was automatically generated by pkg-size-action

@privatenumber
Copy link
Member

As with any change, it's important to outline the problems it's addressing so others can provide better feedback (hence the PR template -- please don't delete it next time).

Right now the PR just describes a hope or an intention. But PRs should address something more concrete.

That said, I'm wondering what specific problem/frustration you're addressing that the current Feature request template doesn't. And if there are any changes that should be made to the Feature request template instead (eg. delete it, rename it to "Component feature request", etc.)

@landondurnan
Copy link
Contributor Author

Updated description with more detail

Copy link
Member

@privatenumber privatenumber left a comment

Choose a reason for hiding this comment

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

  • A lot of these questions (UI states, prior art, theming) seem like they should be answered by the component designers, rather than the engineer tasked with implementing it. Since we're using issue templates now, it shouldn't be hard for non-technical people to fill out. I wonder if it's worth making a "Component request" issue template intended for designers (no API discussion).

  • I think the "Feature request" issue template will be confusing with this change, and should be edited to clarify "Component feature request".

.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
landondurnan and others added 2 commits October 1, 2021 10:29
Co-authored-by: hiroki osame <osame@squareup.com>
More specific description for prior art suggestion.
@privatenumber
Copy link
Member

Chatted with @landondurnan on Slack and I think the next move is to take a step back and publicly discuss the root problem on Issues/Discussion. That way we can get feedback from contributors on what works best for them.


I think the main problem with this template is it's not clear who it's for (engineer vs designer). It has questions for both designers and engineers, and that makes it difficult for one person to answer.

If we want to add a new Issue template for components, let's start by establishing a process that involves PM/designer + engineers so we can split up expected deliverables by target audience.

.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/NEW_COMPONENT.yml Outdated Show resolved Hide resolved
@landondurnan
Copy link
Contributor Author

Discussion here: #158

Responding to some past feedback:

I think the main problem with this template is it's not clear who it's for (engineer vs designer). It has questions for both designers and engineers, and that makes it difficult for one person to answer.

This template is for engineers as they're articulating everything in the design or what aspects of the component are difficult to communicate in static mockups.

If we want to add a new Issue template for components, let's start by establishing a process that involves PM/designer + engineers so we can split up expected deliverables by target audience.

There is a process that exists for multiple designers to collaborate and review with engineers in Figma. This template is intended to synthesize the hand-off from design to engineering so that a larger group of engineers can align exclusively on the code to support the design requirements. This may require additional iterations with design, but the intent is to determine that collectively upon review of the submitted issue.

@privatenumber
Copy link
Member

privatenumber commented Oct 21, 2021

I'm interested in learning how these changes address the feedback we got from contributors in #158. Can you update the PR description with that?

My takeaways from the discussion are:

  • Any question design can answer shouldn't be the engineer's responsibility
    • I'm thinking the "Theme support" section in this template should be for design to answer.
  • Research on 3rd party design systems should be done by design
  • Use checkboxes to check off prerequisites
  • Most importantly, neither of them feel like design is doing their due diligence before handing off components to engineers
    • I think this eng template should exist, but I think it makes more sense to flesh out the template for design to fill out before finalizing the engineering template. Again, looking at this entire process holistically is key. I think contributors would appreciate it if you could do a 5 min rough draft for design to fill out.

My review is only to support the views of contributors so I tagged them directly for review instead.

@ashjtan @Ryguy11 Please approve PR or leave feedback.

@github-actions
Copy link

@github-actions
Copy link

@landondurnan landondurnan dismissed privatenumber’s stale review October 27, 2021 17:01

Suggestions implemented and / or addressed through feedback from others

@github-actions
Copy link

@landondurnan landondurnan merged commit 64f4510 into master Oct 27, 2021
@landondurnan landondurnan deleted the new-component-template branch October 27, 2021 17:04
@github-actions
Copy link

🎉 This PR is included in version 6.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 7.0.0-beta.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants