open-sauced / app

🍕 Insights into your entire open source ecosystem.
https://pizza.new
Apache License 2.0
430 stars 230 forks source link

fix: fixed dropdown styling #4077

Closed ryandotfurrer closed 2 months ago

ryandotfurrer commented 2 months ago

I decreased the size of the modal as well as the dropdown in desktop. However, I increased the size on mobile to make it more inline with the mobile styling.

fix #4063

Description

This PR fixes bug #4063 by decreasing the size of the dropdown selector on desktop. I also took it upon myself to make the mobile styles more inline by increasing the size of the mobile dropdown. You can see these changes in the video below.

Related Tickets & Documents

Fixes #4063

Mobile & Desktop Screenshots/Recordings

Watch the video I made for this fix.

Desktop

Before Arc Repositories  LetsGetTechnicalgridiron-survivor-000195

After Arc Repositories  LetsGetTechnicalgridiron-survivor-000197

Mobile

Before Arc Repositories  LetsGetTechnicalgridiron-survivor-000199

After Arc Repositories  LetsGetTechnicalgridiron-survivor-000201

Steps to QA

Visit any repo of your liking and click the "Add to workspace" button. Please check it in mobile as well.

Tier (staff will fill in)

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

an-older-man-wearing-glasses-and-a-maroon-sweater-is-smiling

netlify[bot] commented 2 months ago

Deploy Preview for oss-insights ready!

Name Link
Latest commit 44986dbf5d2787240e749807896457544b077f19
Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/66d831a9d1dd5d00085b74de
Deploy Preview https://deploy-preview-4077--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.

netlify[bot] commented 2 months ago

Deploy Preview for design-insights ready!

Name Link
Latest commit 44986dbf5d2787240e749807896457544b077f19
Latest deploy log https://app.netlify.com/sites/design-insights/deploys/66d831a9e0016b0008fbbb1c
Deploy Preview https://deploy-preview-4077--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.

ryandotfurrer commented 2 months ago

Thank you for the PR! Two things:

  1. Please attach screenshots for visual bug fixes like these; the PR template has a section!
  2. The mobile view uses a different component called AddToWorkspaceDrawer that requires a similar fix:

@zeucapua I am so sorry about that! I made a video but it wasn't showing up. I've fixed the PR comment to include the link to the video as well as appropriate screenshots.

The way I went about fixing this touched on the single-select component and AddToWorkspaceModal, I did not need to touch the AddToWorkspaceDrawer. Please let me know your thoughts on the fix and how you would have went about it.

nickytonline commented 2 months ago

Just rerunning the E2E tests we have a bit of flakiness in CI at the moment. 😅

nickytonline commented 2 months ago

Going to take a peek at the fix, but just FYI the .env changes haven't been reverted yet. It still shows as a change in the PR.

CleanShot 2024-09-03 at 10 42 39

@ryandotfurrer, if you run git checkout beta -- .env and commit those changes to your branch and push them, the .env file will be removed from your PR.

ryandotfurrer commented 2 months ago

Just rerunning the E2E tests we have a bit of flakiness in CI at the moment. 😅

We've experienced this in GIS as well, no worries 😅

ryandotfurrer commented 2 months ago

Going to take a peek at the fix, but just FYI the .env changes haven't been reverted yet. It still shows as a change in the PR. CleanShot 2024-09-03 at 10 42 39

@ryandotfurrer, if you run git checkout beta -- .env and commit those changes to your branch and push them, the .env file will be removed from your PR.

Done, ty for that! Any idea why the .env file was added in the first place? Just because I made changes to it? I made changes because Iw as trying to solve the /login issue before remembering it's an issue due to GIS.

nickytonline commented 2 months ago

Going to take a peek at the fix, but just FYI the .env changes haven't been reverted yet. It still shows as a change in the PR. CleanShot 2024-09-03 at 10 42 39

@ryandotfurrer, if you run git checkout beta -- .env and commit those changes to your branch and push them, the .env file will be removed from your PR.

Done, ty for that! Any idea why the .env file was added in the first place? Just because I made changes to it? I made changes because Iw as trying to solve the /login issue before remembering it's an issue due to GIS.

It not in the gitignore for our project as it's settings for people to be able to contribute to the project, so any changes made will appear if you git add ..

Maybe longer term we could have an example file and just have a step where we ask people to copy it. Pretty low on the priority list atm though. 😅

ryandotfurrer commented 2 months ago

Looks great in the drawer and in the modal now. Thanks @ryandotfurrer!

Thanks for the collaboration, @nickytonline! I misunderstood the ticket at first but justify-between is an obvious solution 😅

I'll ask for clarification next time if there is any area for confusion on the ticket.

ryandotfurrer commented 2 months ago

Great job! LGTM

Thank you!