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

Merged
merged 8 commits into from
Jul 30, 2024
Merged

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 Jul 30, 2024 1:44pm
nusmods-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 30, 2024 1:44pm

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 line in your changes missing coverage. Please review.

Project coverage is 54.75%. Comparing base (2d4743d) to head (cd217db).
Report is 3 commits behind head on master.

Files Patch % Lines
website/src/views/settings/BetaToggle.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3696      +/-   ##
==========================================
+ Coverage   53.87%   54.75%   +0.87%     
==========================================
  Files         274      274              
  Lines        6032     6043      +11     
  Branches     1449     1452       +3     
==========================================
+ Hits         3250     3309      +59     
+ Misses       2782     2734      -48     

☔ 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 🙏

Comment on lines 27 to 28
white-space: nowrap;
margin: auto;
Copy link
Member

@zwliew zwliew Jul 30, 2024

Choose a reason for hiding this comment

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

I'll remove these style changes because the layout look worse on small form factors.

@zwliew
Copy link
Member

zwliew commented Jul 30, 2024

LGTM, thanks!

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"
3 participants