Revamp Import & Setting - UI Switching #745
Conversation
See Issue Settings Option To Switch User Interface #368
|
sorry for all the feedback! other than the stuff noted above and below, it works well with some light testing locally. permalink pages should not show legacy ui, they should redirect. eg might want to use the SVG version of the logo, attaching here: a few formatting issues: on the import page, at a certain breakpoint the right column wraps over the full-width column. the full-width column goes off the page, and the info bullets aren't showing: on the settings page, more right column breaking strangely. might be nice to have this be 33% up to a certain point and then jump to 50% or 100%. |
|
Hi Josh,
Thanks for the thorough review. Just a few items to finish off on your list. I don’t quite understand the permalink comment “permalink pages should not show legacy ui, they should redirect.”. I’m not sure what you are seeing or how to repro it. A little more help on this one would be appreciated.
Wayne
From: Josh Reisner ***@***.***>
Sent: March 21, 2022 11:24 PM
To: code4recovery/12-step-meeting-list ***@***.***>
Cc: Wayne Parker ***@***.***>; Author ***@***.***>
Subject: Re: [code4recovery/12-step-meeting-list] Revamp Import & Setting - UI Switching (PR #745)
sorry for all the feedback! other than the stuff noted above and below, it works well with some light testing locally.
permalink pages should not show legacy ui, they should redirect. eg /meetings/my-meeting-name should redirect to /meetings?meeting=my-meeting-name. /locations/my-location should either redirect to /meetings or 404.
might want to use the SVG version of the logo, attaching here:
<https://user-images.githubusercontent.com/1551689/159419777-9152fbe2-1070-4e5d-af76-616466aca37c.svg>
a few formatting issues: on the import page, at a certain breakpoint the right column wraps over the full-width column. the full-width column goes off the page, and the info bullets aren't showing:
<https://user-images.githubusercontent.com/1551689/159419802-b5c79b77-f2bd-4d63-9dcb-f5e04d158efb.png>
on the settings page, more right column breaking strangely. might be nice to have this be 33% up to a certain point and then jump to 50% or 100%.
<https://user-images.githubusercontent.com/1551689/159420137-bbaa418b-5a2c-43f3-9f8a-9107ada1560f.png>
—
Reply to this email directly, view it on GitHub <#745 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANT2UM36BA463PWKZYLYV3TVBFRQDANCNFSM5RJP2UUA> .
You are receiving this because you authored the thread. <https://github.com/notifications/beacon/ANT2UM74YCT2R5A5NGBOTYDVBFRQDA5CNFSM5RJP2UUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOIAH6MNY.gif> Message ID: ***@***.*** ***@***.***> >
|
|
I think I can help. When you have tsml_ui selected, and you click on the meeting details, you get something like: My suggestion for how to do this, in the |
|
thanks @brianw-area24 -- good callouts. i'd forgotten about the widgets. @kiyote33 thanks for all the work yesterday, the formatting / wrapping issues are fixed now. looks like some new padding got introduced on the settings pages though. it will look best if the left edge of text can be consistent throughout
also i don't think this bordered appearance is working visually. maybe we should go back to the simple bulleted appearance we had before? finally, i think we might need to split public.scss into two files now. most of the file is related to the TSML frontend, and we don't need to display that when TSML UI is selected -- it could cause unexpected behavior and we will want to deprecate it at the same time as the rest of the code. however there are a few lines at the end that are wordpress-specific that we will want to keep: https://github.com/code4recovery/12-step-meeting-list/blob/master/assets/src/public.scss#L1214-L1268. it would be fine to put this off for a future PR though :) |
|
Here's a view of the Import tab with the vertical meetings summary list and variable card heights. Does this work? |
|
looks a lot better, thanks! there is still some margin strangeness which you may be working on. looks like paragraphs don't have any bottom padding, it would be good to separate these for readability especially in the "where's my info" section. also there's some extra left margin in the description under "import data sources." |
Fixed issue with permalinks. Set summary list to vertical bullets. Removed redundant html from template files. Added widget support. Segregated Import & Settings tab code to their own separate files.
templates/archive-tsml-ui-blocks.php
Outdated
| <?php | ||
| tsml_assets(); | ||
|
|
||
| echo do_blocks( '<!-- wp:template-part {"slug":"header","theme":"twentytwentytwo","tagName":"header","className":"site-header","layout":{"inherit":true}} /-->' ); |
There was a problem hiding this comment.
also here: looks like this adds functionality specific to the twenty twenty two theme. what happens if it's a block theme but not twenty twenty two? should we be checking if that theme is installed? or can we think of a solution for block themes that doesn't require twenty twenty two?
|
when the user makes a change to any of the cards, the confirmation message shows up inside the "General" card -- this seems like it would be preferable to show up in the box that was changed, or above all the boxes
|
|
@brianw-area24 @97eighty @tech2serve @joshreisner Seems like we may be ready for a merge. Can we get everyone signed off before doing so? |
|
@kiyote33 thanks, it's looking good. the only is i see at this point is the block-theme stuff does not seem ready to me. per my comment above, we are assuming that twenty twenty two is installed if we see a block theme. if you install a different one (i tried one called "skatepark" you see no header or footer, and the html is not valid. also i don't think it really fixes twenty twenty two either, see screenshot below. i'd rather not introduce code that only partly fixes one theme and causes a regression in others. my recommendation is to remove twenty twenty two: |
|
but it's fine with me if you want to merge this PR and we as a team can handle it in a future PR. all i'm saying here is i don't think we should ship this block theme support in |
|
That's fine. It definitely does not work right at this time, so I will follow your suggestions. |
Added UI Switch for TSML UI and back to Legacy UI.
Refactored Import Settings page to group settings and features with tabs and cards.
Closes Issue #368