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

Replace deprecated Hidden component with useMediaQuery hook #3246

Merged
merged 15 commits into from
Oct 8, 2024

Conversation

imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Jul 10, 2024

Resolves #3220

The following pages/components have layout changes at various window widths. A review of this pr confirms that the changes are visually the same in this branch and on QA.

  • Landing Page
  • App Bar (best tested with a long project name and a long username)
  • Project Screen
  • Project Settings (just the tabs, not the individual settings)
  • Tree View

Transitions can occur at 600px, 900px, 1200px, 1536px window widths.


This change is Reviewable

@imnasnainaec imnasnainaec added frontend maintenance Issue that makes it difficult to maintain the software or to upgrade installations post-release. labels Jul 10, 2024
@imnasnainaec imnasnainaec self-assigned this Jul 10, 2024
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.

Project coverage is 74.70%. Comparing base (d5916c5) to head (94326c9).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/components/LandingPage/TopBar.tsx 0.00% 2 Missing ⚠️
src/components/LandingPage/index.tsx 0.00% 2 Missing ⚠️
src/components/AppBar/ProjectButtons.tsx 75.00% 0 Missing and 1 partial ⚠️
src/components/AppBar/UserMenu.tsx 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3246      +/-   ##
==========================================
- Coverage   74.71%   74.70%   -0.02%     
==========================================
  Files         280      280              
  Lines       10838    10856      +18     
  Branches     1314     1317       +3     
==========================================
+ Hits         8098     8110      +12     
- Misses       2370     2374       +4     
- Partials      370      372       +2     
Flag Coverage Δ
backend 83.92% <ø> (ø)
frontend 66.63% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@imnasnainaec imnasnainaec marked this pull request as ready for review July 12, 2024 12:54
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewed 14 of 22 files at r1, 9 of 9 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)


src/components/AppBar/ProjectButtons.tsx line 51 at r2 (raw file):

  const navigate = useNavigate();

  const isMdUp = useMediaQuery<Theme>((t) => t.breakpoints.up("md"));

These 't' are so close to the useTranslation 't' visually that I glitched. Should we change it? I don't know.

Copy link
Collaborator Author

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 23 files reviewed, all discussions resolved (waiting on @jasonleenaylor)


src/components/AppBar/ProjectButtons.tsx line 51 at r2 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

These 't' are so close to the useTranslation 't' visually that I glitched. Should we change it? I don't know.

How about th instead?

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 10 of 10 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

@imnasnainaec imnasnainaec merged commit 508758a into master Oct 8, 2024
18 checks passed
@imnasnainaec imnasnainaec deleted the un-hidden branch October 8, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend maintenance Issue that makes it difficult to maintain the software or to upgrade installations post-release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace deprecated Hidden component
2 participants