Add sections to settings #849
Conversation
MohamadNateqi
requested review from
alexmigf,
Terminator-Barbapapa and
BrunoPavlinic98
|
@MohamadNateqi @alexmigf @BrunoPavlinic98 I made a few style tweaks. Let me know what you think. |
|
@Terminator-Barbapapa It looks nice. I find it a little hard to distinguish between the title and content here:
It would be good if you could somehow improve that. |
@Terminator-Barbapapa I think it looks and works great. Regarding the hard distinguish issue @MohamadNateqi mentioned, maybe we could put the Enable option (and all other "standalone" options, if there are any more of them) in their standalone div (the lighter div with gray border), the same one that is being shown when we open a particular section. If not that, we can provide tabulation to it so that standalone options are in the same level as the section grouped options. |
|
I agree that this option seems out of place. I think apart from some changes in the styling we should also look at where the options are placed, as I think we can create a more logical layout. In this case I think the 'Enable' option would fit well in the 'General' section. Like this: 
|
@Terminator-Barbapapa I agree to put the Enable option to general section, and I agree with sliders. |
| $('.settings_category h2' ).click( function() { | ||
| const index = $( '.settings_category h2' ).index( $( this ) ); | ||
|
|
||
| $( this ).toggleClass( 'active' ).next( '.form-table' ).slideToggle( 'fast', function() { |
There was a problem hiding this comment.
This does not work for screens sizes under 960px. At the moment we use display: block !important here:
So the section is never hidden when clicking on the section title. Also the slide animation is not working when the .form-table element is set to display: table;. Which we do when viewing the settings full screen.
There was a problem hiding this comment.
@Terminator-Barbapapa For the full-screen mode, where the display: table;, I think there is no easy way to fix it.
One way would be to make the display property block, then make the transition, and then revert it to table, which has other side effects and issues, such as the bad display of elements during the transition.
Another option would be to switch to height transition instead of toggling the display.
Another option would be wrapping the form-table around a DIV element and working with that, but this is not a good way as it requires many modifications.
I tried to fix it using the first option, but several issues appeared that made me think this was not a good approach.
If we're going to fix this issue for full-screen mode, then switching to height transition might be the better option here.
Would you happen to have any suggestions or opinions?
|
@MohamadNateqi can you fix the conflicts here and also address the comments from @Terminator-Barbapapa ? |
# Conflicts: # assets/css/settings-styles.min.css # assets/js/admin-script.min.js
|
I noticed that the document title and the related settings, which used to be at the top, are now placed below everything. This change feels a bit less intuitive. I have a few suggestions to improve the user experience:
|
close #668
This PR aims to add sections to settings.
Each section is an accordion, as shown in the image. When you first visit the settings page, only the General section is open, while the rest are closed. Any changes made to the state of the accordions will be saved in the localStorage. The localStorage has been chosen to prevent the unnecessary overhead of storing the states in the database and making queries.
For settings that are added by the Professional plugin, two approaches exist:
I implemented the second approach and created a "More" section to keep the rest of the uncategorized settings.
Also, the helpers method has been created to modify sections more easily. To see how sections can be added for the Pro extension, you can switch to this PR's branch: https://github.com/wpovernight/woocommerce-pdf-ips-pro/pull/443.
Since we used the word "section" for another purpose, I used the word "category" in my codes.