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

Release Planner #3696

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wr1159
Copy link
Contributor

@wr1159 wr1159 commented Mar 27, 2024

Context

Resolves #3689

Implementation

  • Display planner in Navbar on all devices (Including mobile devices which didn't show the planner in the past, was it intended or was it a bug?)
  • Remove currentTests in BetaToggle
  • Remove beta feedback for Planner page
Description Before After
No more beta toggle image image
Planner tab in mobile navbar image image
No more leave feedback button image image

Other Information

Since there's no more test, the beta test toggle in settings doesn't appear by default anymore. Is this intended?

Noticed that the plan to take, exemptions and trash section overflows for mobile. Should we fix it first before releasing this to public? We can make another issue for it, because it's UI related I think we need some discussion for it.

Copy link

vercel bot commented Mar 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nusmods-export ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 0:02am
nusmods-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 0:02am

Copy link

vercel bot commented Mar 27, 2024

@wr1159 is attempting to deploy a commit to a Personal Account owned by @nusmodifications on Vercel.

@nusmodifications first needs to authorize it.

Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 53.58%. Comparing base (5774b51) to head (01b7eea).

❗ Current head 01b7eea differs from pull request most recent head 8a50aad. Consider uploading reports for the commit 8a50aad to get more accurate results

Files Patch % Lines
website/src/views/settings/BetaToggle.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3696      +/-   ##
==========================================
- Coverage   53.58%   53.58%   -0.01%     
==========================================
  Files         273      272       -1     
  Lines        5983     5976       -7     
  Branches     1429     1426       -3     
==========================================
- Hits         3206     3202       -4     
+ Misses       2777     2774       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@kokrui kokrui left a comment

Choose a reason for hiding this comment

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

Thanks for the PR (including the screenshots and all)!

I think this PR looks good to me and I approve of it in-principle, but I agree with you that we should fix the mobile UI first before merging this. Thanks for pointing that out!

@kokrui
Copy link
Member

kokrui commented Apr 1, 2024

I'll approve this pending merge conflicts and stuff, but merging will be on hold for now due to some internal requirements with NUS. We expect to be able to merge in around a month or so 🙏

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.

Move planner out of "beta"
2 participants