uncharted-aske / HMI

Apache License 2.0
1 stars 0 forks source link

Settings content adjust to the container size #184

Closed YohannParis closed 3 years ago

YohannParis commented 3 years ago

Why

close #64 the settings of the graph information are too wide.

How

Screenshot

Screen Shot 2021-04-19 at 14 42 41

Testing

Grab and slide the grid, and make sure they do not go below 25% of the screen.

RosaRomeroGomez commented 3 years ago

I think you might have committed your own yarn.lock file by accident @YohannParis?

RosaRomeroGomez commented 3 years ago

I think you might have committed the yarn.lock file by accident @YohannParis ?

YohannParis commented 3 years ago

I think you might have committed the yarn.lock file by accident @YohannParis ?

Yes I did, It wasn't part of the .gitignore so I sent it as well. I'll make sure to remove it.

YohannParis commented 3 years ago

I updated the responsive grid to allow min/max limitation on the cell dimensions. Right now I set them up to { min: 25%, max: 75% }, but this will allow better UX control in the future.

@mj3cheun I'm sorry if I spent too much arguing about your code; it's such a powerful component. I wanted to be 100% sure before touching it.

adamocarolli commented 3 years ago

I think you might have committed the yarn.lock file by accident @YohannParis ?

Yes I did, It wasn't part of the .gitignore so I sent it as well. I'll make sure to remove it.

Adding some thoughts here: yarn.lock should not be part of .gitignore because we do want to check it in to make sure that every developer is running the same environment. Generally it gets updated in two ways:

  1. You've installed a new library for a feature. In this case you should check in your updated yarn.lock with the PR.
  2. Existing dependencies have been updated. If this is the case I'd encourage checking it in as a standalone PR.
mj3cheun commented 3 years ago

2 comments, and I wasn't able to test the resize component properly because of the error above but I looked at the code and I think it should work. Good job, that component is definitely very difficult to work with.

YohannParis commented 3 years ago

Ok @mj3cheun, per our discussion, I removed the min/max widths for the cells. This needs more work to adjust with fixed-width cells, etc.

But I spent time on the setting bar to be more consistent design-wise.

mj3cheun commented 3 years ago

There is an issue here in the knowledge view due to the scientific notation number parsing, we might have to flip the order for the page count around Screen Shot 2021-04-28 at 6 38 25 PM

Edit: Or, maybe if NaN then use original?

YohannParis commented 3 years ago

There is an issue here in the knowledge view due to the scientific notation number parsing, we might have to flip the order for the page count around Screen Shot 2021-04-28 at 6 38 25 PM

Edit: Or, maybe if NaN then use original?

@mj3cheun I update the Counters to be more flexible. I created a type Counter to enforce formatting in the future.

mj3cheun commented 3 years ago

Looks good thanks!