Open abea opened 5 years ago
I'm happy to pick this one up.
This is good to look into. When workflow is present, "trash" is just a flag in the schema and nothing more, the page does not get moved to be a child of the "legacy trashcan" (as I call it when I'm thinking about this), it remains a child of the same parent as before.
However, in the reorganize modal, there is logic to create a "virtual trashcan" as a simulated parent for all the trashed children of a given parent, so the view isn't cluttered by the trash. The user can drag in and out of that trashcan but all they are really doing is toggling the flag.
All of that worked... at one point. It sounds like it still mostly works but there may be CSS issues and/or the styling may just be so unclear that it's not apparent what is supposed to be happening.
On Thu, Sep 26, 2019, 5:07 PM Alex Bea notifications@github.com wrote:
With apos-workflow enabled, the page reorganize modal's Trash rows (when you expand pages with children) isn't aligned to be clear about the relationship. If the Trash row is part of the "Service" page context (in the screenshot), it probably should be aligned with that page's children, possibly shaded red as the "true" trash at the bottom.
[image: Screen_Shot_2019-09-26_at_3_36_26_PM] https://user-images.githubusercontent.com/1155984/65724970-349dd600-e077-11e9-890a-daaeebe6d8a3.png
Some additional indentation work to align pages with children with those that don't have children would probably be good as well.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27LWP7OKGAJRUFPHJULQLUQALA5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HN75N5Q, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27OPSQMILWTQ23F3G2LQLUQALANCNFSM4I27CXMA .
One of the major reasons why it works this way is that otherwise, dragging a parent page out of the trash would result in all of its children no longer being in the trash — a change to each of the pages. Now, what happens if only some of those changes, to some of those pages, get committed?
However, relatively recently, workflow was modified to allow the page tree to differ between two locales, or between draft and live; movements in the page tree now have to be committed, and they have the right ripple effects (i.e. the children move too when you commit this change to the parent).
That means the door is now open to a single trashcan even with workflow.
There are other nice things about the current behavior though, mainly that the traditional trash can is a bit of a mess - I want to bring this page back, but where the heck is it in the trash, and where did I need to move it back to? Tough to tell. With the "trash stays in place" behavior, you just toggle the trash flag (or drag it out of the virtual trashcan), there's no mystery as to where it will wind up or where it came from.
On Fri, Sep 27, 2019 at 8:06 AM Tom Boutell tom@apostrophecms.com wrote:
This is good to look into. When workflow is present, "trash" is just a flag in the schema and nothing more, the page does not get moved to be a child of the "legacy trashcan" (as I call it when I'm thinking about this), it remains a child of the same parent as before.
However, in the reorganize modal, there is logic to create a "virtual trashcan" as a simulated parent for all the trashed children of a given parent, so the view isn't cluttered by the trash. The user can drag in and out of that trashcan but all they are really doing is toggling the flag.
All of that worked... at one point. It sounds like it still mostly works but there may be CSS issues and/or the styling may just be so unclear that it's not apparent what is supposed to be happening.
On Thu, Sep 26, 2019, 5:07 PM Alex Bea notifications@github.com wrote:
With apos-workflow enabled, the page reorganize modal's Trash rows (when you expand pages with children) isn't aligned to be clear about the relationship. If the Trash row is part of the "Service" page context (in the screenshot), it probably should be aligned with that page's children, possibly shaded red as the "true" trash at the bottom.
[image: Screen_Shot_2019-09-26_at_3_36_26_PM] https://user-images.githubusercontent.com/1155984/65724970-349dd600-e077-11e9-890a-daaeebe6d8a3.png
Some additional indentation work to align pages with children with those that don't have children would probably be good as well.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27LWP7OKGAJRUFPHJULQLUQALA5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HN75N5Q, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27OPSQMILWTQ23F3G2LQLUQALANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
Is there a guide available on how to get a local instance running? I have looked over the documentation and I can only see how to use apostrophe, not so much on running the code.
Are you asking how to get your first Apostrophe project running? Or how to get our apostrophe-enterprise-testbed project running in order to work on this workflow-related ticket? My apologies if you're working with one of our clients and I've completely misread what level you're coming into this at!
The basics are here:
https://docs.apostrophecms.org/apostrophe/tutorials/getting-started/setting-up-your-environment https://docs.apostrophecms.org/apostrophe/tutorials/getting-started/creating-your-first-project
This ticket is something of an advanced one because it involves our workflow module, documentation here:
https://github.com/apostrophecms/apostrophe-workflow
The enterprise testbed project is here, it's not pretty because we mostly use it for regression testing but it's handy in that it has workflow and some locales set up out of the box:
https://github.com/apostrophecms/apostrophe-enterprise-testbed
On Tue, Oct 1, 2019 at 6:25 AM chriswade notifications@github.com wrote:
Is there a guide available on how to get a local instance running? I have looked over the documentation and I can only see how to use apostrophe, not so much on running the code.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27MJUDDCUDLMQNV52ITQMMQR3A5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAAZA7Y#issuecomment-536973439, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27NX54Y7ZTQQG67IEDDQMMQR3ANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
A brand new apostrophe project without workflow will not exhibit the bug discussed in this ticket.
On Tue, Oct 1, 2019 at 10:16 AM Tom Boutell tom@apostrophecms.com wrote:
Are you asking how to get your first Apostrophe project running? Or how to get our apostrophe-enterprise-testbed project running in order to work on this workflow-related ticket? My apologies if you're working with one of our clients and I've completely misread what level you're coming into this at!
The basics are here:
https://docs.apostrophecms.org/apostrophe/tutorials/getting-started/setting-up-your-environment
https://docs.apostrophecms.org/apostrophe/tutorials/getting-started/creating-your-first-project
This ticket is something of an advanced one because it involves our workflow module, documentation here:
https://github.com/apostrophecms/apostrophe-workflow
The enterprise testbed project is here, it's not pretty because we mostly use it for regression testing but it's handy in that it has workflow and some locales set up out of the box:
https://github.com/apostrophecms/apostrophe-enterprise-testbed
On Tue, Oct 1, 2019 at 6:25 AM chriswade notifications@github.com wrote:
Is there a guide available on how to get a local instance running? I have looked over the documentation and I can only see how to use apostrophe, not so much on running the code.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27MJUDDCUDLMQNV52ITQMMQR3A5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAAZA7Y#issuecomment-536973439, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27NX54Y7ZTQQG67IEDDQMMQR3ANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
Thanks for the info @boutell. I have never used Apostrophe before and came across this issue when browsing open-source. It looked like a simple enough issue so I thought I would jump on it. I may still be able to get around to working on it but it will take me some time. Please assign to someone more experienced if there are time constraints.
No problem Chris! I suggest you dive into the Apostrophe tutorials — I'm sure you'll contribute in another way if not here. Are you participating in Hacktoberfest perhaps?
On Tue, Oct 1, 2019 at 7:12 PM chriswade notifications@github.com wrote:
Thanks for the info @boutell https://github.com/boutell. I have never used Apostrophe before and came across this issue when browsing open-source. It looked like a simple enough issue so I thought I would jump on it. I may still be able to get around to working on it but it will take me some time. Please assign to someone more experienced if there are time constraints.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27K4TKHKLIKLKIPGLNTQMPKPHA5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEADBIVY#issuecomment-537269335, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAH27I2DW77YUB44X2FOWDQMPKPHANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
Just to be clear, you're proposing indenting the Trash at the end of the parents children and shading it red like the true Trash at the bottom?
Yes that would make sense. I think it was the case once.
On Mon, Oct 14, 2019 at 2:45 AM Kingsley Torlowei notifications@github.com wrote:
Just to be clear, you're proposing indenting the Trash at the end of the parents children and shading it red like the true Trash at the bottom?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27OOYQ3BZHGNCJ7NHCTQOQIP7A5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBDPLUQ#issuecomment-541521362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27P3QNIKQ26B66JT6BDQOQIP7ANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
(Also the "true trash" is a pretty useless concept on a site that always had workflow. When the trashInContext flag is present it should really be hidden entirely unless it has legacy contents.)
On Mon, Oct 14, 2019 at 7:53 AM Tom Boutell tom@apostrophecms.com wrote:
Yes that would make sense. I think it was the case once.
On Mon, Oct 14, 2019 at 2:45 AM Kingsley Torlowei < notifications@github.com> wrote:
Just to be clear, you're proposing indenting the Trash at the end of the parents children and shading it red like the true Trash at the bottom?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/2045?email_source=notifications&email_token=AAAH27OOYQ3BZHGNCJ7NHCTQOQIP7A5CNFSM4I27CXMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBDPLUQ#issuecomment-541521362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27P3QNIKQ26B66JT6BDQOQIP7ANCNFSM4I27CXMA .
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
-- Chief Software Architect Apostrophe Technologies Pronouns: he / him / his
It looks and feels like the current Trash is indented and shaded red. So i feel like this current implementation works just fine. Can this issue be closed for now?
This was more about the sub-section "trash" rows. Are those fixed?
Would something like this be appropriate?
That looks much better!
I currently have a pull request to fix this issue. #2089
We should check if #2089 should be reverted. The main trash is no longer flush left as it was before.
For acceptance, the main trash should be unchanged, but when using apos-workflow, the nested trash sections should be aligned with children of the parent page (or similarly indented), then items within those nested trash sections should be indented normally as well. (all indentations should be the same size)
Hey can I work on this, I think, I may help in this.
Hi @themayank-goyal - You can give it a go, but this issue is for an old version of Apostrophe that we are maintaining, but not really updating that much.
Where can i get the emails to ask for the clarification of this issue. I am a newbie in the opensource contribution.
Where can i get the emails to ask for the clarification of this issue. I am a newbie in the opensource contribution.
Not sure what you mean by this. You can discuss either here or in our Discord (https://discord.com/invite/XkbRNq7). This issue is for someone who knows the Apostrophe 2 code base really well. I'm not sure how much JavaScript experience you have, but this isn't an easy change.
can i start on this ?
@yogeshsingh2672000 Sure! I would read the comments above first. Have you worked on an Apostrophe 2 project previously? As I commented above, this isn't a clear-cut problem and requires some experience. Otherwise, it will be a lot of work just getting set-up to diagnose and replicate the error. Cheers, Bob
can i start on this? i think i can resolve this
@aditya162002 Sure! Just read through the comments above first. Again, this is an Apostrophe 2 bug. Cheers, Bob
I would like to give it a try. Can I start work on this?
Hi @shokerm - Anything in our public repos is open for contributions. Like I cautioned the others, read back through the comments and understand it is an Apostrophe 2 bug. Cheers, Bob
hey @shokerm are you still working on this issue can i help in any way
hi @Harshitahusts you are welcome to give it a try.
Please keep in mind this is all for a very old version of Apostrophe, so if you are eager to contribute to versions that are used in new projects, it is best to work on 3.x issues.
On the other hand, if you're an A2 user and this bugs you... feel free to address it!
please assign this issue to me @abea
Hi @dxfuryman, This issue is still open, so you are free to work on it. I would read the comments above first. Have you worked on an Apostrophe 2 project previously? As I commented above, this isn't a clear-cut problem and requires some experience. It is also on an old version of Apostrophe. It will be a lot of work just getting set-up to diagnose and replicate the error. Cheers, Bob
@BoDonkey It might be a good idea to remove the "help wanted" and "good first issue" tags. They might be misleading at this point.
@abea - good point! LOL! I didn't even look at the labels.
With apos-workflow enabled, the page reorganize modal's Trash rows (when you expand pages with children) isn't aligned to be clear about the relationship. If the Trash row is part of the "Service" page context (in the screenshot), it probably should be aligned with that page's children, possibly shaded red as the "true" trash at the bottom.
Some additional indentation work to align pages with children with those that don't have children would probably be good as well.