Closed KathJohnson closed 8 years ago
@KathJohnson @1sp I think those questions can better be answered by Sudhir. I can, however, offer some advice.
1) On the social icons, those are housed inside the sidebar menu code. Maybe there should be a toggle for displaying them in either the header or footer? I could place them in the footer or header using CSS on his sites, if needed.
2) On the sidebar menu for light theme, I've never liked the sidebar and think it should go away entirely, but that's not up to me. It is redundant and takes up valuable real estate, like he said. One fix might be to get rid of the sidebar and change the Bootstrap class to col-md-12 (I think it is, off the top of my head) for full-width in the content area and move the social as recommended in #1. Another way is to keep it and make it so you can disable it in the admin settings then conditionally add the bootstrap class to make it full-with. I'm not sure which is more difficult, Sudhir can tell us how long it would take to do those two options.
I think this is also the issue with New England Soundproofing, the sidebar seems to bother him as well, although he hasn't articulated it as well as the heirloom windows client.
1) As John rightly said, the social icons could go either in header or footer. The exact placement in the section could be discussed. From a user's perspective, I would prefer having a admin setting to control the placement of social icons
2) This again could get a place in settings page as "Enable sidebar". If disabled the content area will take 100% container width
Ok, thanks, Sudhir. Any idea of the timeframe on those two options? Since we already have the sidebar on several sites, we probably have to keep it, so making it optional in the settings is probably best. Please advise as to the number of hours for these so we can budget for it. Thanks you.
On Apr 28, 2016, at 1:08 PM, Sudhir notifications@github.com wrote:
1) As John rightly said, the social icons could go either in header or footer. The exact placement in the section could be discussed. From a user's perspective, I would prefer having a admin setting to control the placement of social icons
2) This again could get a place in settings page as "Enable sidebar". If disabled the content area will take 100% container width
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-215497167
For the (1) I would need to know the placement of the the social media icon section in header/footer. If it is not something tricky , I can wrap both 1. and 2 within couple of hours time
@sp1 I'd say that if the left nav is suppressed that social media icon bar should go to the footer. It should default to centered. @jboz62 Would that be easily editable in HTML, then, in case we need to left-justify it or something? I'm just trying to foresee likely issues with that.
Sincerely, Katherine Johnson, Research Director Corbett Research Group http://www.restorationtrades.com "Web Marketing that Works"
Publishers of:
The Custom Building & Restoration Trades Directory http://www.restorationtradesdirectory.com
The Building Arts Notebook, "Journal of the Guild of Building Artisans" http://buildingartisansguild.com/
Email: katherine@restorationtrades.com Phone: (413) 475-3154
On Thu, Apr 28, 2016 at 2:17 PM, Sudhir notifications@github.com wrote:
For the (1) I would need to know the placement of the the social media icon section. If it is not something tricky , I can wrap both 1. and 2 within couple of hours time
— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-215516652
That sounds like a good plan to me. Moving the social icons around, centered, left or right is easy with CSS once they are placed with the footer, not a problem. Default to centered and I can move them left or right if needed for a particular design.
On Apr 28, 2016, at 2:27 PM, John M. Corbett notifications@github.com wrote:
@sp1 I'd say that if the left nav is suppressed that social media icon bar should go to the footer. It should default to centered. @jboz62 Would that be easily editable in HTML, then, in case we need to left-justify it or something? I'm just trying to foresee likely issues with that.
Sincerely, Katherine Johnson, Research Director Corbett Research Group http://www.restorationtrades.com "Web Marketing that Works"
Publishers of:
The Custom Building & Restoration Trades Directory http://www.restorationtradesdirectory.com
The Building Arts Notebook, "Journal of the Guild of Building Artisans" http://buildingartisansguild.com/
Email: katherine@restorationtrades.com Phone: (413) 475-3154
On Thu, Apr 28, 2016 at 2:17 PM, Sudhir notifications@github.com wrote:
For the (1) I would need to know the placement of the the social media icon section. If it is not something tricky , I can wrap both 1. and 2 within couple of hours time
— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-215516652
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-215519524
@KathJohnson Are we a go on this, Katherine? Let Sudhir know if he should work on this please. Thank you.
@1sp I just spoke with John Corbett about this, and yes, this is a go. Please proceed, Sudhir. Thanks.
The functionality is working. Tested on the light theme.
That looks great, Sudhir, thank you. Is that on a branch we can test?
On May 4, 2016, at 11:45 AM, Sudhir notifications@github.com wrote:
The functionality is working. Tested on the light theme.
Admin setting https://cloud.githubusercontent.com/assets/60359/15019817/308bc90c-123d-11e6-999e-2aca04f31db7.png Left nav disabled https://cloud.githubusercontent.com/assets/60359/15019845/497d0e08-123d-11e6-9260-f48ebf5a3536.png Left nav enabled https://cloud.githubusercontent.com/assets/60359/15019859/55b66f0c-123d-11e6-8ebf-b200aad95f9a.png — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-216907602
Yes, it is on the same i221 branch.
I have implemented and tested it on pages for light theme (wasn't sure about the dark theme).
That is good news, and the screenshots look good. I deployed the code from branch i221 to the test site menus.corbettresearchgroup.com. but I don't see the new checkbox on the Admin Settings page.
I’m not seeing it either, and the home page item is missing.
On May 4, 2016, at 2:31 PM, KathJohnson notifications@github.com wrote:
That is good news, and the screenshots look good. I deployed the code from branch i221 to the test site menus.corbettresearchgroup.com. but I don't see the new checkbox on the Admin Settings page.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-216958239
This is how it would look like, are all the migrations in place ?
please share admin login of http://menus.corbettresearchgroup.com
I thought I sent you the admin login, they are all the same login. Let me know if you have that. Thanks.
On May 4, 2016, at 2:40 PM, Sudhir notifications@github.com wrote:
please share admin login of http://menus.corbettresearchgroup.com http://menus.corbettresearchgroup.com/ — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-216960807
I believe the latest changes are not on the site. Please confirm if changes till commit#/fdd94c39c0140b2e402f8377606acca5bfa5148c ( https://github.com/1sp/cms/commit/fdd94c39c0140b2e402f8377606acca5bfa5148c ) are deployed
There are 3 migrations , don't miss them :)
Sudhir, any idea why the migrations don’t run if there’s a “php artisan migrate” in the script? It should run them, shouldn’t it?
On May 4, 2016, at 2:48 PM, Sudhir notifications@github.com wrote:
There are a couple of migrations , don't miss them :)
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-216963264
Actually it is not about the migration, even the changes to the templates (e.g the new checkbox setting) were missing. So I am guessing that my commit wasn't pulled.
It doesn’t look like anything was migrated:
Wed May 4 14:26:46 EDT 2016 Already on 'i221' Your branch is up-to-date with 'origin/i221'. From github.com:meetinghouse/cms
php artisan clear-compiled php artisan optimize Generating optimized class loader php artisan cms:setup Nothing to migrate.
On May 4, 2016, at 2:48 PM, Sudhir notifications@github.com wrote:
There are a couple of migrations , don't miss them :)
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-216963264
Hmm, it has a git pull origin $BRANCH and $BRANCH is set to i221.
Ah, I think you are pulling from the meetinghouse's repo instead of mine. I haven't made a pull request so the the commit lies on my repository.
You can add a new remote to your copy, say 'sudhir ' and point it to my fork and then do a git pull sudhir i221
Do you mean the previous pull request you made? I don't see any current pull requests.
I'll try the remote branch you mentioned, thank you.
Please ignore my previous message.I just merged 1sp/i221 into meetinghouse/i221 myself. Just do a regular pull again and you will see my changes
This setup looks great, Sudhir, thank you for the work on that!
I ran into an error while trying to create a page, also get the same error when trying to edit an existing page. Please see screenshot.
Another issue I'm seeing is that we lost our "active"/"not-active" classes on the top and sidebar menu. The reason for that is that Request::server('PATH_INFO')
doesn't seem to work. We figured out that $_SERVER['REQUEST_URI']
does work, so we changed most of them, you may have seen it in the sidebar.dark file and top-nav file. Can you get those working again? Looks like it's missing from the light theme sidebar, please add it in there as well. I'll need those classes for styling the menus.
You can see it working on http://lightthemetest.corbettresearchgroup.com/ on the top nav, should also be working on the sidebar nav. You can also see it on the sidebar nav for http://shconstruction2.corbettresearchgroup.com/about (dark theme)
I'm going to need this functionality for this site on the top nav and sidebar nav, but it needs to be available for all sites: http://newenglandsoundproofing.corbettresearchgroup.com/
### This is the contact page, you can see it says "not-active" when it is the active page.
### This is from a working site where the active page has the "active" class:
Thank you.
@1sp Ok, one last thing, I hope ;)
When you disable the left menu and update the settings, the menu remains visible until you logout of the admin or browse away by clicking on a menu item. This might be confusing to some users. Can we add to the message under the checkbox that says "Use this to enable or disable left navigation." something like "If you disable the left sidebar menu, it will remain visible while in the admin. To view the full-width page or post, please click on a menu item". Either that, or we should just make in not display after update in the admin. @KathJohnson What do you think about that, Katherine?
@1sp Sudhir, maybe this will be the last one :)
Noticed one more problem while I while fixing another issue. The left menu on the dark theme can be disabled. Not sure if this is a problem or not, Katherine can decide. I'm thinking this should only work for the light theme, not on the dark theme as it only has one menu. Maybe this functionality should be separated so that the disable checkbox only works on the light theme? Thank you. (EDITED)
@1sp Guess there are still some bugs to work out. One more thing... On the dark theme menu, when you click on "home" or "contact" (single pages with no sub-items) they are repeated in the sub-nav area below. Those should not be there, see screenshots. Thank you.
It's probably easier to show the menu in the admin pages and just put some verbiage in there to explain that it won't show on the live pages. So I'd do that.
I'm able to suppress the menu on the menus.corbettresearchgroup.com site now, but I don't see the social icon bar anywhere when it's suppressed. Should it be in the footer?
Shows up for me, maybe clear your cache? All the way at the bottom:
@KathJohnson Looks like Sudhir is busy on some other jobs on Upwork, probably why he hasn't responded today. We need him to fix the bugs so we can push this out to some of the sites soon. We need it on the Heirloom and WindowLogic site and also the New England Soundproofing site so I can do the changes for the mockup.
@1sp Sudhir, when you get these bugs worked out, can you do a final pull request so we can merge this into into "blog". Please push this through as soon as you're able. Thank you.
Sorry @jboz62 , @KathJohnson , seems like my last commit disrupted a lot of things. I will be more careful from now on :)
I am pushing a commit with fixes as soon as possible.
Many, many thanks to both of your for your attention to detail on this. I do agree, John, that it needs to go live soon, and that is the client site list I am working form as well. Of course, the remaining issues are a priority.
I just merged changes in https://github.com/meetinghouse/cms/tree/1sp-in . Please review
Ah, never mind my last email, I see. Once this is reviewed it will be merged into “blog”? I’ll take a look at it later today.
On May 6, 2016, at 1:10 PM, Sudhir notifications@github.com wrote:
I just merged changes in https://github.com/meetinghouse/cms/tree/1sp-in https://github.com/meetinghouse/cms/tree/1sp-in . Please review
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-217501547
Sudhir, the changes look good, but still seeing some things that were missed in my previous comments. The "active" and "not-active" classes are still not working. Please see above comments for the fix on that. In this screenshot, the Contact page is active, but the class says "not-active".
This is also not working on the dark theme, which will break the shconstuction.com site if we push the code to it, so this needs to be fixed before we merge to "blog".
I can add these changes if you don't have time, but I have too much going on already. If you can do it, please do. Thank you.
The other thing I noticed is with the message on the sidebar menu, should be changed to notify users that sidebar will be visible in admin and not visible on front-end of site. This is not urgent, though, I can add this later. Thank you.
Katherine, the bug fixes Sudhir did look good, just one more thing to fix with the menu classes “active” and "not-active”, they are not working still. I mentioned this to Sudhir and hopefully he can get it fixed soon. Once that’s working, this should be ready to merge to “blog” and pushed out to the sites we discussed. If he can get it done this weekend, I will have him merge it and then I can deploy to new england sound so I can get those changes done for Steve.
On May 6, 2016, at 9:34 AM, KathJohnson notifications@github.com wrote:
Many, many thanks to both of your for your attention to detail on this. I do agree, John, that it needs to go live soon, and that is the client site list I am working form as well. Of course, the remaining issues are a priority. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-217442101
I just pushed a fix for the above. The menus should look a lot better now :)
On light theme, the parent menu items in left navigation get active class in their h4 tag
Please let me know if anything else need a fix
I see the social icons now. I was looking for them in the editable footer field on the Admin Settings page, but I suppose they would need to be edited by you, John, not by the user. That's fine. I think the functionality of suppressing the left nav is good at this point, though we're still testing the other menu management pieces.
If it's possible to roll out just the left nav suppression piece to the blog branch, then I think we probably can do that.
Yes, that’s not an item that would be editable in the footer, it’s dynamically generated. I can move it around some using CSS, though, if needed.
All changes have been merged to blog already. I’ve been working with it and haven’t seen any issues so far, except what you mentioned, but that could be a missing migration table.
On May 10, 2016, at 9:49 AM, John M. Corbett notifications@github.com wrote:
I see the social icons now. I was looking for them in the editable footer field on the Admin Settings page, but I suppose they would need to be edited by you, John, not by the user. That's fine. I think the functionality of suppressing the left nav is good at this point, though we're still testing the other menu management pieces.
If it's possible to roll out just the left nav suppression piece to the blog branch, then I think we probably can do that. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-218162885
I was behind in my understanding of where we are on rolling this out. Do you mind if I close this issue? I think it is working.
Yes, sorry, I wanted to wait, but didn’t hear back from you, so we merged it. If needed it is possible to roll back changes to an earlier state. However, I do think it’s working pretty well, except for the bug you found. Let’s see what Sudhir comes back with.
I would try doing another deploy on lightthemetest to see if that frees it up, you may be missing the last migration table he added.
On May 10, 2016, at 10:08 AM, John M. Corbett notifications@github.com wrote:
I was behind in my understanding of where we are on rolling this out. Do you mind if I close this issue? I think it is working. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-218168318
Don't apologize. I am very happy to see how productive you've been.
By the way, if you prefer to hand Steve Drago back off to me, that would be fine. Once you think the technical side of things is sort of ironed out, I will work with him on the content.
Sincerely, Katherine Johnson, Research Director Corbett Research Group http://www.restorationtrades.com "Web Marketing that Works"
Publishers of:
The Custom Building & Restoration Trades Directory http://www.restorationtradesdirectory.com
The Building Arts Notebook, "Journal of the Guild of Building Artisans" http://buildingartisansguild.com/
Email: katherine@restorationtrades.com Phone: (413) 475-3154
On Tue, May 10, 2016 at 10:13 AM, jboz62 notifications@github.com wrote:
Yes, sorry, I wanted to wait, but didn’t hear back from you, so we merged it. If needed it is possible to roll back changes to an earlier state. However, I do think it’s working pretty well, except for the bug you found. Let’s see what Sudhir comes back with.
I would try doing another deploy on lightthemetest to see if that frees it up, you may be missing the last migration table he added.
On May 10, 2016, at 10:08 AM, John M. Corbett notifications@github.com wrote:
I was behind in my understanding of where we are on rolling this out. Do you mind if I close this issue? I think it is working. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub < https://github.com/meetinghouse/cms/issues/243#issuecomment-218168318>
— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-218170050
That’s fine, if you want to work with him on content, I think we’re pretty much there, but I’m sure there will be tweaks he wants. One he already wants is to change “portfolios” to “projects”, but I don’t think that’s possible without changing the code base. He also wanted to hide “all projects” in the menu, not sure why that’s showing up, maybe another thing for Sudhir to address. I put it at the end and hid it. We also hid the top menu. You can see the changes on the site.
I give you a ring soon.
On May 10, 2016, at 10:22 AM, John M. Corbett notifications@github.com wrote:
Don't apologize. I am very happy to see how productive you've been.
By the way, if you prefer to hand Steve Drago back off to me, that would be fine. Once you think the technical side of things is sort of ironed out, I will work with him on the content.
Sincerely, Katherine Johnson, Research Director Corbett Research Group http://www.restorationtrades.com "Web Marketing that Works"
Publishers of:
The Custom Building & Restoration Trades Directory http://www.restorationtradesdirectory.com
The Building Arts Notebook, "Journal of the Guild of Building Artisans" http://buildingartisansguild.com/
Email: katherine@restorationtrades.com Phone: (413) 475-3154
On Tue, May 10, 2016 at 10:13 AM, jboz62 notifications@github.com wrote:
Yes, sorry, I wanted to wait, but didn’t hear back from you, so we merged it. If needed it is possible to roll back changes to an earlier state. However, I do think it’s working pretty well, except for the bug you found. Let’s see what Sudhir comes back with.
I would try doing another deploy on lightthemetest to see if that frees it up, you may be missing the last migration table he added.
On May 10, 2016, at 10:08 AM, John M. Corbett notifications@github.com wrote:
I was behind in my understanding of where we are on rolling this out. Do you mind if I close this issue? I think it is working. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub < https://github.com/meetinghouse/cms/issues/243#issuecomment-218168318>
— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-218170050
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/meetinghouse/cms/issues/243#issuecomment-218172713
@1sp Sudhir, it looks like there's a bug remaining on this feature. When you click on one of the portfolio items on light theme, the sidebar menu reappears. See: http://heirloomwindows.corbettresearchgroup.com/exterior
Thank you.
Sorry, the last fix was only done for the pages resources. I have now done the same change for posts, portfolios and projects resources. Changes have been merged in blog branch. Please review.
Thanks
@jboz62 I think you mentioned you expected this to maybe come up, but here's a note I got in response to the client for heirloomwindows and weatherstrippingkit
"The one thing that REALLY concerns me is the inefficient use of space. I think I've mentioned this to John. On both sites, the first thing that jumps out at me is that 25% of my prime real estate is dedicate to redundant information. The same links are on the left side bar as are just above in the menu bar. This shifts all the copy to the right and gives it an unsettling, unbalanced feeling. If we have to have the social media links, put them in the header or footer, but don't take up valuable copy space."
So, three questions, which you can answer in terms of estimated hours, if you do find them feasible: