Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance form layout for readability #1323

Closed
netniV opened this issue Feb 8, 2018 · 31 comments
Closed

Enhance form layout for readability #1323

netniV opened this issue Feb 8, 2018 · 31 comments
Labels
enhancement General tag for an enhancement

Comments

@netniV
Copy link
Member

netniV commented Feb 8, 2018

I think the Change Settings screen is a little unintuitive because it forces you to tick the box below the option that you are wanting to set. Worse, you might miss the fact that you have to tick that box because you can change the setting and continue on down the screen.

The enhancement to this would be to moving the tick box to the left of the option text, and disabling the value entry so that nothing can be changed until that option has been ticked. This would also remove the need to repeat the "Update this field" text making the entire set of options a lot shorter and more visible on the screen at the same time.

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Screen shots will help your description a bit. I'm at a loss. Could be my current state of mind. Not sure.

@cigamit cigamit added the enhancement General tag for an enhancement label Feb 9, 2018
@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

OK, just mocking some up, should help explain it.
screen shot 2018-02-09 at 00 34 29

change settings 1

Here's the proposal, but I do have an image I'm making that shows the final layout

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

I'm good with that, can you have it done by Sunday? Rony will do 1.1.35 on Sunday.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

image

Don't no, think it will have to wait for 1.1.36 or later. I'm working solid this weekend unless something changes on Sunday.

I can do the layout change easy enough, but it'll take me a bit longer to work on the JQ as that will need a class or something to work from to enable the value controls

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Need a title attribute __('Update this field');

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

Over the checkbox, or over the disabled controls, or both?

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Checkbox I think. Or if the whole div, then __('Click the checkbox to update this field');

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

I might be able to knock it out in a few minutes. I can not remember if this thing is some of that old code where everything was done by hand, or if it's controlled by an API. Let me take a peak.

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Okay, this is an API change. So 'might' be easy. Depends on whether or not it can be done without CSS changes.

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Was not all that difficult...

@cigamit cigamit changed the title Enhance the Change Settings screen Enhance form layout for readability Feb 9, 2018
cigamit added a commit that referenced this issue Feb 9, 2018
Enhance form layout for readability
@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Okay, done. Being that it's leveraging the API, bot the Data Templates and the Graph Templates benefited from this change also. Makes the pages quite a bit more compressed.
changedevicesettings
graphtemplates
datatemplates

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Close if good.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

The only thing I think it needs is to disable the value input unless checked. But I haven’t tested yet. Will do later.

@anarkia1976
Copy link

Very good guys, now it's superclean. thanks.

netniV added a commit to netniV/cacti that referenced this issue Feb 9, 2018
@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

So I have been so impressed with the above, I decided to add the disable options myself. However, this does require a CSS and JS change (unless you have a functions somewhere else that does the same job).

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

@anarkia1976 Do you fancy testing this latest patch? You should be able to see a visual difference in the greyed out input fields and the inability to actually click on them.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

Meh, just thought I'd hunt around to see the differences on different sections. It all looks good until I found a problem with these changes in graph_template_inputs.php (probably just needs the api updating):

screen shot 2018-02-09 at 09 35 22

@anarkia1976
Copy link

Sorry, but until monday i can't test new patches. :)

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

This is why we need to test more before release ;)

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

The Graph Item Inputs. That whole screen has been a thorn in my side for a decade. It's very confusing, but was important in the past as it helped you 'fix' some things, but no one should ever have to see it. It's dangerous. I'll fix this new issue, but we really need to think about making it less incomprehensible.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

On the testing front, I was reading about a headless chrome that you can automate through the website. If we created a script that produced screenshots and notified about differences in layouts at the end of a day when there was development change (timezone?) that could also help.

cigamit added a commit that referenced this issue Feb 9, 2018
Additional changes to clean up layout of graph_templates_inputs.php
cigamit added a commit that referenced this issue Feb 9, 2018
@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Well, that would be so awesome!

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Okay, I've got the graph_templates_inputs.php squared away. Converted the Associated Graph Items to a table to take up less space.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

The styling that I proposed on #1325 does work against this form. Not sure if that matters or not.

@cigamit
Copy link
Member

cigamit commented Feb 9, 2018

Show me some screen shot's of what you propose, I'm off to the day job. We have till Sunday night EST to work things out.

@netniV
Copy link
Member Author

netniV commented Feb 9, 2018

I merely mean that it doesn't disable the input field. Also, the onChange of the checkbox probably isn't assigned since it's writing it's own checkbox. I am assuming that the checkbox is to enable use of the field to the left on data_template_input? If not, may need some table headers for explanation.

@cigamit
Copy link
Member

cigamit commented Feb 10, 2018

Okay, I've implemented this in host.php specifically. We can think of a more generic approach for 1.2. Look over how I implemented this and feel free to continue the discussion here for now. Can you see the projects tab on the GitHub gui by the way. We should be opening projects for large scale gui improvement there. It's better to have those conversations there.

@cigamit cigamit closed this as completed Feb 10, 2018
@netniV
Copy link
Member Author

netniV commented Feb 10, 2018

Yes, I can see there are two projects open there.

@cigamit
Copy link
Member

cigamit commented Feb 10, 2018

Great, if you come across additional ideas, and you can create projects, start creating them there. We can first kick the can there, and then open the issue once we have figured out the right path.

@netniV
Copy link
Member Author

netniV commented Feb 10, 2018

Oh, whilst I can see, I can't create, modify etc.

cigamit added a commit that referenced this issue Feb 11, 2018
Some themes require visibility on the checkbox labels.  So, add more
control through CSS.
@cigamit
Copy link
Member

cigamit commented Feb 11, 2018

Okay, I'll mention that to Rony tomorrow.

cigamit added a commit that referenced this issue Feb 11, 2018
Some themes require visibility on the checkbox labels.  So, add more
control through CSS.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement General tag for an enhancement
Projects
None yet
Development

No branches or pull requests

3 participants