AmpliPi icon indicating copy to clipboard operation
AmpliPi copied to clipboard

Settings UI Fixes

Open SteveMicroNova opened this issue 1 year ago β€’ 4 comments
trafficstars

What does this change intend to accomplish?

Working on a handful of issues relating to setting page UI concerns, as seen in the 0.4.0 project board UI/UX section

Intends to close #554 and #498

Checklist

  • [x] Have you tested your changes and ensured they work?
  • [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • [x] If applicable, have you updated the CHANGELOG?
  • [x] Does your submission pass linting & tests? You can test on localhost using ./scripts/test
  • [x] If this is a UI change, have you tested it across multiple browser platforms?
  • [x] If this is a UI change, have you tested across multiple viewport sizes (ie. desktop versus mobile)?

SteveMicroNova avatar Jun 24 '24 14:06 SteveMicroNova

This PR is ready for review pending an answer to this question on #498

SteveMicroNova avatar Jun 28 '24 16:06 SteveMicroNova

This code is currently deployed to lamplipi, if you wish to see it while reviewing. You should not see a difference when switching between inhouse and lamplipi on the settings pages now., see image at top of the thread to see what was solved by replacing the List and ListItem components

SteveMicroNova avatar Jun 28 '24 16:06 SteveMicroNova

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.93%. Comparing base (b516bb7) to head (d9bcd10). Report is 155 commits behind head on main.

:exclamation: Current head d9bcd10 differs from pull request most recent head 9359cd0

Please upload reports for the commit 9359cd0 to get more accurate results.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
+ Coverage   50.90%   50.93%   +0.03%     
==========================================
  Files          25       26       +1     
  Lines        5838     6549     +711     
==========================================
+ Hits         2972     3336     +364     
- Misses       2866     3213     +347     
Flag Coverage Ξ”
unittests 50.93% <ΓΈ> (+0.03%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jun 28 '24 17:06 codecov-commenter

I think this PR introduced #777 ; can you please stylize these consistently? inconsistent font families makes the app feel unpolished.

Could you point to an example of this issue within this branch? The fonts look the same to me, maybe just with a different weight for the header vs the others but no outright different fonts afaik

SteveMicroNova avatar Jun 28 '24 20:06 SteveMicroNova

does this fix https://github.com/micro-nova/AmpliPi/issues/833 too?

Let me deploy this and check real quick, if it does I'll connect that issue before merging so it gets closed as well

SteveMicroNova avatar Aug 08 '24 19:08 SteveMicroNova

Let me deploy this and check real quick, if it does I'll connect that issue before merging so it gets closed as well

No

SteveMicroNova avatar Aug 08 '24 19:08 SteveMicroNova