micro-nova / AmpliPi

Whole House Audio System 🔊
https://amplipi.com
GNU General Public License v3.0
282 stars 23 forks source link

Convert TextField into MUI TextField on Stream Settings Modal #717

Open SteveMicroNova opened 3 months ago

SteveMicroNova commented 3 months ago

https://github.com/micro-nova/AmpliPi/pull/716#discussion_r1596950395

We recreate almost blow for blow an element that is in a library we're already using, but with reduced functionality. Stylize the mui version to be the same as ours and replace it

rtertiaer commented 3 months ago

what does this work gain us right now? I'd argue that until we have a need to use some functionality within MUI, this doesn't need to happen; in other words, this will happen the moment it solves a problem and probably isn't worth our time any sooner. In & of itself, it is not an issue.

SteveMicroNova commented 3 months ago

what does this work gain us right now?

It gains us convenience moving forward, every time I have to screw with this modal I forget that we made a thing with the literal exact same name as another thing that we use all over the place and get confused when tooling for that component doesn't work with the one that's here and my first instinct is always to see if there's some sort of styling in parent components that is overwriting what I'm trying to do rather than to assume that we've remade something from the ground up and given it the same name

Maybe now that it's severely annoyed me twice now I'll remember it for next time, but there is a legitimate lost time sink due to having this component be not the same as the one of the same name we use across multiple other projects and even elsewhere within amplipi

rtertiaer commented 3 months ago

if it's causing you hassle while you're trying to solve a problem that a user experiences, that is the perfect opportunity to accomplish this task. However, what we have right now functions and doesn't break in production; until that isn't true, this ticket is a solution in search of a problem.