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

feat: Made entire row for selectable table clickable. #177

Merged
merged 5 commits into from
Aug 12, 2022

Conversation

chadstewart
Copy link
Contributor

@chadstewart chadstewart commented Aug 11, 2022

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Description

This PR adds the feature of making the entire row clickable in the selectable table component.

Related Tickets & Documents

Fixes #84

Mobile & Desktop Screenshots/Recordings

localhost_6006__path=_story_design-system-molecules-selectable-table--default (4) (1)

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@chadstewart chadstewart marked this pull request as ready for review August 11, 2022 15:22
@netlify
Copy link

netlify bot commented Aug 11, 2022

βœ… Deploy Preview for oss-insights ready!

Name Link
πŸ”¨ Latest commit ef56487
πŸ” Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/62f556302e43b00008aeb6e0
😎 Deploy Preview https://deploy-preview-177--oss-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Aug 11, 2022

βœ… Deploy Preview for design-insights ready!

Name Link
πŸ”¨ Latest commit ef56487
πŸ” Latest deploy log https://app.netlify.com/sites/design-insights/deploys/62f55630e7d2840008ba88cc
😎 Deploy Preview https://deploy-preview-177--design-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@chadstewart chadstewart changed the title Made entire row for selectable table clickable. feat: Made entire row for selectable table clickable. Aug 11, 2022
@bdougie
Copy link
Member

bdougie commented Aug 11, 2022

The click area doesn't extend to the whole row. Per the issue @pixelsbyeryc, I was thinking that what was expected. Also curious if the hover state should highlight the row as it is pictured. Might be helpful for the user.

image

Copy link
Contributor

@0-vortex 0-vortex left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@eryc-cc eryc-cc left a comment

Choose a reason for hiding this comment

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

☝️ One more thing.

Add a border radius to that blue area.

LGTM otherwise πŸ•

@chadstewart
Copy link
Contributor Author

point_up One more thing.

Add a border radius to that blue area.

LGTM otherwise pizza

Added the border but I don't like it. Causes all the content to shift and makes it look kinda janky.

@eryc-cc
Copy link
Contributor

eryc-cc commented Aug 11, 2022

point_up One more thing.

Add a border radius to that blue area.

LGTM otherwise pizza

Added the border but I don't like it. Causes all the content to shift and makes it look kinda janky.

No need to add a border, just the border-radius, I think in tailwind is rounded

Maybe rounded-sm? Should be 4-6px max... more than that will look too rounded

@chadstewart
Copy link
Contributor Author

No need to add a border, just the border-radius, I think in tailwind is rounded

Maybe rounded-sm? Should be 4-6px max... more than that will look too rounded

Added the border radius but it doesn't seem to affect the highlight. I'm using background color for the highlight so that might be the issue.

Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

Looks good, let's ship it.

@0-vortex 0-vortex merged commit 8fb914a into beta Aug 12, 2022
@0-vortex 0-vortex deleted the 84-make-entire-row-selectable branch August 12, 2022 01:42
github-actions bot pushed a commit that referenced this pull request Aug 12, 2022
## [1.1.0-beta.2](v1.1.0-beta.1...v1.1.0-beta.2) (2022-08-12)

### πŸ• Features

* add animation to the progress pie component ([#182](#182)) ([90f4bbb](90f4bbb)), closes [#126](#126)
* make entire row for selectable table clickable ([#177](#177)) ([8fb914a](8fb914a)), closes [#84](#84)
@github-actions
Copy link
Contributor

πŸŽ‰ This PR is included in version 1.1.0-beta.2 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

github-actions bot pushed a commit that referenced this pull request Aug 13, 2022
## [1.1.0](v1.0.5...v1.1.0) (2022-08-13)

### βœ… Tests

* enable parallel netlify build for storybook ([#170](#170)) ([d6c5a75](d6c5a75)), closes [#49](#49)

### πŸ” Continuous Integration

* add better release target deployment urls ([#172](#172)) ([f3a93f0](f3a93f0))

### πŸ§‘β€πŸ’» Code Refactoring

* update login text to say connect with github. ([#178](#178)) ([5b7e572](5b7e572)), closes [#139](#139)

### πŸ• Features

* add animation to the progress pie component ([#182](#182)) ([90f4bbb](90f4bbb)), closes [#126](#126)
* make entire row for selectable table clickable ([#177](#177)) ([8fb914a](8fb914a)), closes [#84](#84)
* Simplify the Dashboard ([#155](#155)) ([80d445c](80d445c))

### πŸ› Bug Fixes

* correct domain dot com bubble error ([#173](#173)) ([3816edb](3816edb))
* Fixing 'Maximum Depth Reached' or Infinite Loop bug. ([#188](#188)) ([d7a4954](d7a4954))
@github-actions
Copy link
Contributor

πŸŽ‰ This PR is included in version 1.1.0 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

0-vortex added a commit that referenced this pull request Aug 15, 2022
* origin/main:
  chore(patch): release 1.1.2 [skip ci]
  fix(auth): enable auth with SSR and API routes (#196)
  chore(patch): release 1.1.1 [skip ci]
  fix(ui): broken tools nav (#194)
  chore(minor): release 1.1.0 [skip ci]
  chore(patch): release 1.1.0-beta.3 on beta channel [skip ci]
  fix: Fixing 'Maximum Depth Reached' or Infinite Loop bug. (#188)
  chore(patch): release 1.0.5 [skip ci]
  fix: Delete codeql-analysis.yml (#186)
  chore(minor): release 1.1.0-beta.2 on beta channel [skip ci]
  feat: make entire row for selectable table clickable (#177)
  feat: add animation to the progress pie component (#182)
  chore(minor): release 1.1.0-beta.1 on beta channel [skip ci]
  feat: Simplify the Dashboard (#155)
  chore(patch): release 1.0.4 [skip ci]
  fix: Make the description relevant (#180)
  chore(patch): release 1.0.4-beta.3 on beta channel [skip ci]
  refactor: update login text to say connect with github. (#178)
  chore(patch): release 1.0.4-beta.2 on beta channel [skip ci]
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.1.0-beta.2](open-sauced/app@v1.1.0-beta.1...v1.1.0-beta.2) (2022-08-12)

### πŸ• Features

* add animation to the progress pie component ([#182](open-sauced/app#182)) ([90f4bbb](open-sauced/app@90f4bbb)), closes [#126](open-sauced/app#126)
* make entire row for selectable table clickable ([#177](open-sauced/app#177)) ([8fb914a](open-sauced/app@8fb914a)), closes [#84](open-sauced/app#84)
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.1.0](open-sauced/app@v1.0.5...v1.1.0) (2022-08-13)

### βœ… Tests

* enable parallel netlify build for storybook ([#170](open-sauced/app#170)) ([d6c5a75](open-sauced/app@d6c5a75)), closes [#49](open-sauced/app#49)

### πŸ” Continuous Integration

* add better release target deployment urls ([#172](open-sauced/app#172)) ([f3a93f0](open-sauced/app@f3a93f0))

### πŸ§‘β€πŸ’» Code Refactoring

* update login text to say connect with github. ([#178](open-sauced/app#178)) ([5b7e572](open-sauced/app@5b7e572)), closes [#139](open-sauced/app#139)

### πŸ• Features

* add animation to the progress pie component ([#182](open-sauced/app#182)) ([90f4bbb](open-sauced/app@90f4bbb)), closes [#126](open-sauced/app#126)
* make entire row for selectable table clickable ([#177](open-sauced/app#177)) ([8fb914a](open-sauced/app@8fb914a)), closes [#84](open-sauced/app#84)
* Simplify the Dashboard ([#155](open-sauced/app#155)) ([80d445c](open-sauced/app@80d445c))

### πŸ› Bug Fixes

* correct domain dot com bubble error ([#173](open-sauced/app#173)) ([3816edb](open-sauced/app@3816edb))
* Fixing 'Maximum Depth Reached' or Infinite Loop bug. ([#188](open-sauced/app#188)) ([d7a4954](open-sauced/app@d7a4954))
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.

4 participants