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: create list feature #1698

Merged
merged 36 commits into from
Sep 21, 2023
Merged

feat: create list feature #1698

merged 36 commits into from
Sep 21, 2023

Conversation

OgDev-01
Copy link
Contributor

@OgDev-01 OgDev-01 commented Sep 12, 2023

Description

This pull request includes various features, fixes, and refactors to enhance the functionality and user experience of the Insights hub. It introduces the "hub list" feature, adds fetch hooks for lists and contributors, implements a filter chip component, and creates a contributor hub header component. Additionally, it includes several bug fixes, style updates, and code cleanups. Overall, these changes aim to improve the performance, layout, and navigation of the Insights hub.

Things to take note of

  • The range filter is not functional yet but will be added in my next PR
  • limit filter works
  • the other advanced filters will be coming up in follow up PRs

coming up next

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

Related Tickets & Documents

related to #1665 #1663

Mobile & Desktop Screenshots/Recordings

Components

image
Screen.Recording.2023-09-14.at.19.08.31.mov

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?

@netlify
Copy link

netlify bot commented Sep 12, 2023

Deploy Preview for design-insights ready!

Name Link
🔨 Latest commit c2eb8e2
🔍 Latest deploy log https://app.netlify.com/sites/design-insights/deploys/650b8d0c61e17600088d2a9c
😎 Deploy Preview https://deploy-preview-1698--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 configuration.

@netlify
Copy link

netlify bot commented Sep 12, 2023

Deploy Preview for oss-insights ready!

Name Link
🔨 Latest commit c2eb8e2
🔍 Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/650b8d0ceb2b0a0008cc8941
😎 Deploy Preview https://deploy-preview-1698--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 configuration.

@OgDev-01
Copy link
Contributor Author

Side note:

I pulled changes from #1682 to expedite implementation while it undergoes review.

@brandonroberts brandonroberts added requested changes and removed needs review PRs that need review from core engineering team labels Sep 19, 2023
@OgDev-01
Copy link
Contributor Author

When text is entered into the search bar, the "X" button causes the bar to "jump". Preferably, this would be smooth. For example, on Twitter when searching, the "X" button just appears within the search bar on the right without resizing the search bar itself

Nice catch 👍 . However, fixing the search close icon is out of the scope of this PR. I'll hide the search for now since search is not implemented from the API atm. Opened a ticket for that here #1717

@jpmcb
Copy link
Member

jpmcb commented Sep 19, 2023

Sounds good! The only thing from me then is the overlapping titles in Firefox!

I noticed that it's only happening with longer list names:
Screenshot 2023-09-19 at 2 24 22 PM

But not short ones:
Screenshot 2023-09-19 at 2 23 24 PM

@OgDev-01
Copy link
Contributor Author

Sounds good! The only thing from me then is the overlapping titles in Firefox!

I noticed that it's only happening with longer list names: Screenshot 2023-09-19 at 2 24 22 PM

But not short ones: Screenshot 2023-09-19 at 2 23 24 PM

This has been fixed in the most recent updates.

When the dropdown menu is opened, it causes the scroll bar to disappear which shifts the page over to the right (maybe related to #1696 ?)

this fix is still in progress for Select... hopefully it is merged soon radix-ui/primitives#2253. We can move forward with this until the fix is ready to imlement

@OgDev-01 OgDev-01 self-assigned this Sep 20, 2023
@OgDev-01 OgDev-01 requested a review from jpmcb September 20, 2023 13:03
@OgDev-01
Copy link
Contributor Author

Marking this as blocked till #1718 is merged. I need the multi-select component to implement some basic filters here

@brandonroberts
Copy link
Contributor

@OgDev-01 Let's go ahead and get the page landed so we can reuse the contributors table in #1710. We can add filtering separately.

@OgDev-01 OgDev-01 added needs review PRs that need review from core engineering team and removed blocked labels Sep 20, 2023
@OgDev-01 OgDev-01 removed their assignment Sep 20, 2023
@OgDev-01
Copy link
Contributor Author

Aside adding filters, This is ready for review cc: @open-sauced/engineering

Copy link
Contributor

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

Needs more work on mobile, but its functional

@OgDev-01
Copy link
Contributor Author

Will it be out of scope if I add a Delete flow for the list?

Screen.Recording.2023-09-21.at.01.07.47.mov

Similar to what was done with insights pages

Copy link
Member

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Overall looks great! 👏🏼


I love the idea of a delete user flow. Typing DELETE works 👍🏼 I've also seen where you have to type the name of the resource your deleting to confirm it. I.e., if my list is named "My list", i'd have to type literally "My list" into the prompt to confirm the delete.

But I think if we need to get this PR in, we should look at that as a followup feature add.

@brandonroberts brandonroberts merged commit 7adfe94 into beta Sep 21, 2023
@brandonroberts brandonroberts deleted the 1665-explore-contributors branch September 21, 2023 14:07
open-sauced bot pushed a commit that referenced this pull request Sep 21, 2023
## [1.66.0-beta.1](v1.65.0...v1.66.0-beta.1) (2023-09-21)

### 🍕 Features

* add new`MultiSelect`component to design system ([#1718](#1718)) ([6405b43](6405b43))
* create list feature ([#1698](#1698)) ([7adfe94](7adfe94))
open-sauced bot pushed a commit that referenced this pull request Sep 28, 2023
## [1.66.0](v1.65.0...v1.66.0) (2023-09-28)

### 🍕 Features

* add `delete` functionality to list feature ([#1732](#1732)) ([8a3e1da](8a3e1da))
* add feature flag integration ([#1707](#1707)) ([2bfcf4c](2bfcf4c))
* add new`MultiSelect`component to design system ([#1718](#1718)) ([6405b43](6405b43))
* add User Search ([#1678](#1678)) ([008bf88](008bf88))
* create list feature ([#1698](#1698)) ([7adfe94](7adfe94))

### 🐛 Bug Fixes

* adjust button positioning for Firefox compatibility ([#1730](#1730)) ([91389ba](91389ba))
* adjust style to prevent shift ([#1754](#1754)) ([9328ab8](9328ab8))
* allow not onboarded users to interact with `insights` page ([#1722](#1722)) ([b37ce4d](b37ce4d))
* Cherry-pick 🍒 Update X icon ([#1759](#1759)) ([0a43e6a](0a43e6a))
* fix search bar bug ([#1735](#1735)) ([d16e93f](d16e93f))
* fixed a typo in 30 days label ([#1755](#1755)) ([2293dc2](2293dc2))
* make date selector visible on highlight creation ([#1761](#1761)) ([7b4014c](7b4014c))
* update field mapping for contributor lists create ([#1745](#1745)) ([71b704a](71b704a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review PRs that need review from core engineering team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants