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

Studio: Create new Previews tab and apply design changes #846

Merged
merged 54 commits into from
Jan 30, 2025

Conversation

katinthehatsite
Copy link
Contributor

@katinthehatsite katinthehatsite commented Jan 27, 2025

Related issues

Closes https://github.com/Automattic/dotcom-forge/issues/10277
Closes https://github.com/Automattic/dotcom-forge/issues/10354
Closes https://github.com/Automattic/dotcom-forge/issues/10356

Proposed Changes

This PR adds the design for the quick deploys according to Figma:

Screenshot 2025-01-28 at 4 22 26 PM Screenshot 2025-01-28 at 4 22 49 PM Screenshot 2025-01-28 at 4 23 44 PM

Before testing, please note the things that this PR DOES NOT do (there is already issues opened for these items):

  • It does not handle the expired state for the site and removing an expired demo site
  • It does not have updated state (green checkmark once the user runs Update)
  • It does not have Rename functionality (the button to Rename currently does nothing)
  • It does not handle offline mode consistently
  • The table does not yet do any sorting
  • We would need to check how to handle cases for when the update button should be blocked and when it had previously tooltips (there is issue open already)

Testing Instructions

  • Pull the changes from this branch
  • Start the app with the feature flag STUDIO_QUICK_DEPLOYS=true npm start
  • Navigate to the Previews tab
  • Confirm that you can see different screens e.g. when there is no previews, with one preview, with multiple preview sites
  • Confirm that the design is consistent with what you can see in the Figma linked above
  • Click on the preview actions
  • Try deleting a preview link and confirm it gets deleted
  • Try updating a preview link and confirm the correct preview links updates
  • Don't try the rename button as it is not implemented yet

Notes

  • I tried using DataViews since it was mentioned in the design post (and I have a draft PR with it somewhat working shall we want to decide to use it) but I thought it was somewhat an overkill for what we need. It also requires installing an extra dependency and we would likely need to create a separate CSS file instead of using Tailwind approach that we already have. Even though it has some sorting present by default, I think it is something that we can fairly easily achieve on our end, considering that we could theoretically have 10 data items per one site.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@katinthehatsite katinthehatsite self-assigned this Jan 27, 2025
@katinthehatsite katinthehatsite changed the title Studio: Apply design changes to Repurpose Demo Sites Studio: Create new Previews tab and apply design changes Jan 27, 2025
@wojtekn wojtekn marked this pull request as draft January 28, 2025 07:13
@katinthehatsite
Copy link
Contributor Author

@wojtekn the issue was solved in 4b8919b
Let me know if you are still seeing further problems with this!

@ivan-ottinger ivan-ottinger self-requested a review January 30, 2025 09:44
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment

Choose a reason for hiding this comment

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

Great work, Kat!

I haven't finished the review yet, but noticed the following issue which I would like to share early:

When a new site is being created, it displays on the list as if it was already created:

Screen.Capture.on.2025-01-30.at.10-43-07.mov

It is likely related to the following behavior as well - where it looks like other local sites are generating preview sites as well (even when they are not):

Screen.Capture.on.2025-01-30.at.10-52-53.mp4

@katinthehatsite
Copy link
Contributor Author

Thanks for pointing it out @ivan-ottinger - I have now addressed this in 7cbf41c where I noticed a small issue with filtering snapshots. Feel free to pull the most recent changes and let me know if you can still see the same problem.

@ivan-ottinger
Copy link
Contributor

Thanks for pointing it out @ivan-ottinger - I have now addressed this in 7cbf41c where I noticed a small issue with filtering snapshots. Feel free to pull the most recent changes and let me know if you can still see the same problem.

Nice! Thank you for fixing that. I cannot reproduce the issue anymore. 🙂

Copy link
Contributor

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

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

@katinthehatsite works well now. 🎉

src/components/content-tab-previews.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment

Choose a reason for hiding this comment

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

Thank you for your work on this PR, Kat! I think it has moved the project forward a lot!

I haven't noticed any other functional issues. All the features are working well in my testing (creating, deleting, updating of preview sites; state when there are no preview sites, logged-out WPCOM user, no internet connection). Formatting of the RTL version is looking good as well.

I have shared a couple of ideas and thoughts in the inline comments below.

src/components/content-tab-previews.tsx Outdated Show resolved Hide resolved
src/modules/preview-site/components/preview-site-row.tsx Outdated Show resolved Hide resolved
src/modules/preview-site/components/preview-site-row.tsx Outdated Show resolved Hide resolved
src/components/content-tab-previews.tsx Outdated Show resolved Hide resolved
src/components/content-tab-previews.tsx Show resolved Hide resolved
@ivan-ottinger ivan-ottinger self-requested a review January 30, 2025 11:44
Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

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

I tested it and everything worked as expected.

Ivan made great suggestions, let's address them and merge the PR when ready.

We can create new issues for small tweaks in the UI.

@katinthehatsite katinthehatsite merged commit 43ccb3a into trunk Jan 30, 2025
7 checks passed
@katinthehatsite katinthehatsite deleted the fix/implement-design-for-quick-deploys branch January 30, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants