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

fix: Mobile screen rotation fix #1194

Merged
merged 29 commits into from
Feb 4, 2025

Conversation

KyleKlus
Copy link
Contributor

This pr fixes the current visual bug, when the plugin is used on a mobile device in landscape mode (See pictures).
Further more this pr also adds a "how to develop & test ui changes" to the contribution page, so that these issues wont arise again in a future ui change.

PC design


For reference: Old design used on a pc


New design used on a pc

Mobile design


For reference: Old design on a mobile device in landscape mode with the visual bug


New design on a mobile device in landscape mode


Old design on a small mobile device in landscape mode with visual bug


New design on a small mobile device in landscape mode

New contributing text

@KyleKlus KyleKlus marked this pull request as draft December 28, 2024 14:34
Copy link

codecov bot commented Dec 28, 2024

Codecov Report

Attention: Patch coverage is 96.11650% with 4 lines in your changes missing coverage. Please review.

Project coverage is 93.20%. Comparing base (b2daf9d) to head (df6eb2b).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/question-postponement-list.ts 33.33% 4 Missing ⚠️

❌ Your patch status has failed because the patch coverage (96.11%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1194      +/-   ##
==========================================
+ Coverage   92.89%   93.20%   +0.30%     
==========================================
  Files          42       42              
  Lines        4574     4662      +88     
  Branches      647      374     -273     
==========================================
+ Hits         4249     4345      +96     
+ Misses        318      315       -3     
+ Partials        7        2       -5     

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

@KyleKlus KyleKlus marked this pull request as ready for review December 28, 2024 14:52
@KyleKlus
Copy link
Contributor Author

KyleKlus commented Dec 28, 2024

The thought process behind the end design

The whole thing broke because I didnt check the mobile landscape mode 🙄, when I developed the more detailed session progress title text and didnt assume that someone moved everything around so dramatically.
Someone wanted to make room in the mobile landscape mode and has thus moved all the buttons to the side, to open up some space in the middle (see #990 & #998).

To fix this, I moved the progress text to the card context area, shrunk down the response buttons for small screens and only moved the upper control buttons to the left side in the landscape mode, such that more space is opened up for viewing the cards. I did my best to respect the wish to have more viewing space and at the same time to have a good and consistant design.

PS.: The new contributing text is there so that no one repeats my mistakes 😅

@st3v3nmw st3v3nmw self-requested a review December 31, 2024 08:04
@KyleKlus
Copy link
Contributor Author

KyleKlus commented Jan 14, 2025

The last commit changes the looks of the deck progress info a tiny bit(see pictures below).

Changes:

  • Deck progress info now with bold font weight(highlights the usefullness of the information better):


  • Chosen deck progress info now without sub deck counter if there are no subdecks (removes redundant information for a better focus):

@bentoner
Copy link

@KyleKlus Thanks for fixing this. Works for me on iPad and Desktop.

Copy link
Owner

@st3v3nmw st3v3nmw left a comment

Choose a reason for hiding this comment

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

Hi @KyleKlus, thanks for the fix!

My only question is, can we make the card context always appear on a separate line on its own? Something like this:

image

Currently, it moves to the bottom when the card context is too long which is a bit distracting if it keeps switching places:

image

@KyleKlus
Copy link
Contributor Author

KyleKlus commented Feb 2, 2025

Sure, let's do that 😊.

I'll work on it either this evening or tomorrow, so it should be ready until Tuesday.

Kyle Klus and others added 11 commits February 3, 2025 22:45
This reverts commit 5e84528.
This reverts commit b5e80ed, reversing
changes made to a015ce7.
Change Decks list card count box size

Changed the minimum width of the boxes, that there will fit for numbers up to 9999. Currently there only will fit if the numbers are below 999 what in my case and certainly for others too is not enough.
* Update tr.ts

Türkçe yeni çeviriler eklendi ve bazıları güncellendi.

* Update tr.ts

* Update tr.ts

düzeltmeler ve güncelemeler yapıldı
Copy link
Owner

@st3v3nmw st3v3nmw left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the UI fixes!

@st3v3nmw st3v3nmw merged commit c0f96f7 into st3v3nmw:master Feb 4, 2025
2 of 3 checks passed
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.

7 participants