search

F4IF / ctree-demo

Collaboration Tree - A community discussion platform built for maximum insight with minimum oversight.
http://demo.ctree.org
Other
37 stars 21 forks source link

#10 #47

Closed ThilinaTCH closed 6 years ago

ThilinaTCH commented 6 years ago

10 Add cTree (drawer) search support

CSchmitz81 commented 6 years ago

OK, I reviewed the functionality and the toggle button didn't show up. Also the X button never disappeared. I added a css style so hidden would work for the X button, but the toggle never showed up. Even if it had, that's not the style we wanted anyway (we use icons everywhere), so I updated it to use a bookmark icon. I didn't change the color, but feel free to add that and fix the tooltip I tried to add (didn't work, but if you can't fix it quickly don't worry about it). What I left for you is to tie up the logic to lookup the ctree when the bookmark button is clicked and toggle the saved state. That should finish up this issue.

ThilinaTCH commented 6 years ago

Hi Chris

Everything working fine for me, I guess there is an issue with the polymer versions we are using. Can you try by updating polymer.-cli version

On Mon, Dec 25, 2017 at 5:32 AM Chris Schmitz notifications@github.com wrote:

OK, I reviewed the functionality and the toggle button didn't show up. Also the X button never disappeared. I added a css style so hidden would work for the X button, but the toggle never showed up. Even if it had, that's not the style we wanted anyway (we use icons everywhere), so I updated it to use a bookmark icon. I didn't change the color, but feel free to add that and fix the tooltip I tried to add (didn't work, but if you can't fix it quickly don't worry about it). What I left for you is to tie up the logic to lookup the ctree when the bookmark button is clicked and toggle the saved state. That should finish up this issue.

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/F4IF/ctree-demo/pull/47#issuecomment-353808440, or mute the thread https://github.com/notifications/unsubscribe-auth/ADdTvo1sR-RqG8sPJ_jy4V6cJjd4rppwks5tDuYxgaJpZM4Q54KK .

-- Thanks and Regards, Thilina

CSchmitz81 commented 6 years ago

OK, maybe it not working with your paper-toggle-button was an issue with my CLI. That said, we don't use paper-toggle-button because it doesn't fit with the style, which is why I updated it to paper-icon-button (to match our other bookmark icons). That's why this required implementing on-tap. That said, after looking again at what you did, it was cleaner and appeared to actually fulfill the requirements of the issue, so I added a new component to work like the paper-toggle-button (ctree-toggle-button) and completed this implementation. Please use (and update if needed) ctree-toggle-button instead of paper-toggle-button for any future changes unless you've confirmed using paper-toggle-button with me first.

Now that this is working I'll approve and merge.

ThilinaTCH commented 6 years ago

Hi Chris,

Ok will use ctree-toggle-button if needed Great to hear that my PR is approved

Thanks Thilina

On Fri, Dec 29, 2017 at 12:53 PM, Chris Schmitz notifications@github.com wrote:

OK, maybe it not working with your paper-toggle-button was an issue with my CLI. That said, we don't use paper-toggle-button because it doesn't fit with the style, which is why I updated it to paper-icon-button (to match our other bookmark icons). That's why this required implementing on-tap. That said, after looking again at what you did, it was cleaner and appeared to actually fulfill the requirements of the issue, so I added a new component to work like the paper-toggle-button (ctree-toggle-button) and completed this implementation. Please use (and update if needed) ctree-toggle-button instead of paper-toggle-button for any future changes unless you've confirmed using paper-toggle-button with me first.

Now that this is working I'll approve and merge.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/F4IF/ctree-demo/pull/47#issuecomment-354397815, or mute the thread https://github.com/notifications/unsubscribe-auth/ADdTvoVLn8tX9fnzBkCdUGrI2n06u6Thks5tFHBfgaJpZM4Q54KK .

-- Thanks and Regards, Thilina