klequis / private-money-client

5 stars 2 forks source link

Improve layout #21

Closed dhaef closed 3 years ago

dhaef commented 3 years ago
Screen Shot 2020-12-19 at 7 50 56 PM

Could potentially move the Actions text inputs to there own line in-order to give more room to make them larger and add labels.

klequis commented 3 years ago

I'm giving this some thought but on layout and coding style. Will get back to you hopefully later today.

klequis commented 3 years ago

Thank you for the pull request @dheaf and my apologies for not having had the time to share this information before your changes. Please feel free to ask me as many questions as you have.

What has become the current app as you see it was was originally envisioned as a NodeJS script to run some rules and clean-up data and then a play-ground for different approaches to doing things. It incrementally became what it is over time and as a result has a bit of messiness to it. Naming conventions have been inconsistent but are improving. I originally used Bootstrap, which I usually don't, and then started using styled-components on top of it.

Consistency has been improving and I did a major change today to improve it which is now in both the dev & master branches. Given that, here is how I would like to proceed.

  1. Pull the latest from the 'dev' branch.

  2. The RenderCount component and its output (e.g., 'Rule - total:4 return:4') is debug and I'm not sure I'll be using that anymore. I turned its output off. Within components you can comment it out or just delete it if the component is getting too busy.

  3. Use styled-components for styling

For example, in DatePicker.js

Currently there is:

<DatePickerDiv className="mx-1">

Which can be:

const DatePickerDiv = styled.div`
  margin-bottom: 0.25rem; // this line was copied from bootstrap.css
`

In this case

const DatePickerDiv = styled.div``

was already in the code waiting to have styles added to it. But that isn't always the case. You can add new styled components where needed.

  1. styled-components naming

If you have a styled component Header and then use it later in the component where it is nested with React components, it is some over-head to know that Header is a styled component and not a React component. To get around this let's add to the styled component name the underlying element type. So assuming header is a <div> its name should be HeaderDiv. If it were a <h2> its name should be HeaderH2. On the other hand, if Header was a React component that returned a <h2> it would be named Header.

import React from 'react'

export const Header = ({ headerText }) => {
  return <h2>{headerText}</h2>
}

As to your question on putting the Action inputs on separate lines, I think removing the debug output from RenderCount makes one line adequate for a desktop layout.

I would like to keep the vertical height of Rule as small as possible because, as the user is changing the Criteria, seeing more of CriteriaResults saves scrolling. (If you want to discuss doing a mobile layout I'm happy to, but personally I won't use financial data on my phone so I haven't put thought into it.)

I looked through my actual banking date and found the longest entry for description is 81 characters and takes-up 725px. Making description in Criteria that wide will make the date in a date criterion pretty ugly. However, editing the description without being able to see it gets very tedious, so it is likely worth a little ugliness. If 725px is too wide then shorten it some as it isn't necessary to see all of every description.

I made a drawing on how I would layout the elements on the page. It is 'what goes where' and not at all showing proportions.

rule-edit

klequis commented 3 years ago

Resolved in another pull request. See issue #11