jaykallen / searchapi Goto Github PK

As it is now, making the text the only scrollable area feels very weird. Good improvement over it not being scrollable though 👍 .

CardView isn't meant to be used this way. That said, it's a clever way to solve the problem of making SearchView look more material, and I'd recommend leaving a comment as to why you did it.

I suggest using a dependency injection singleton approach instead. Otherwise, include a well-reasoned comment about why Singleton makes sense.

Most seasoned developers would look at this as a code smell.

If you want to wrap a TextView in an elevated layout, you could very easily wrap it with a FrameLayout that has a background and add elevation.

Whether a company asks for it or not, I highly recommend adding unit tests to your code challenge.

It shows that you understand how to write well-written, and highly maintainable software.

There are times when ConstraintLayout makes sense. It's usually as a replacement to RelativeLayout. In this layout, you could easily use LinearLayout instead and get a performance bump.

It's not the biggest deal in the world, but good to keep in mind.

Instead, use something like

view?.let {
 ...
}

When doing any code challenge, try to avoid leaving behind commented out code, or code that doesn't do anything.

You're creating a circular dependency by referencing the HomeFragment. Additionally, it will make your ViewModel much harder to unit test.