apostrophe icon indicating copy to clipboard operation
apostrophe copied to clipboard

Sectioned trash is not clear visually

Open abea opened this issue 6 years ago • 35 comments

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.

Screen_Shot_2019-09-26_at_3_36_26_PM

Some additional indentation work to align pages with children with those that don't have children would probably be good as well.

abea avatar Sep 26 '19 21:09 abea

I'm happy to pick this one up.

chriswade avatar Sep 26 '19 23:09 chriswade

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 [email protected] 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 .

boutell avatar Sep 27 '19 12:09 boutell

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 [email protected] 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 [email protected] 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

boutell avatar Sep 27 '19 12:09 boutell

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.

chriswade avatar Oct 01 '19 10:10 chriswade

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 [email protected] 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

boutell avatar Oct 01 '19 14:10 boutell

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 [email protected] 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 [email protected] 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

boutell avatar Oct 01 '19 14:10 boutell

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.

chriswade avatar Oct 01 '19 23:10 chriswade

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 [email protected] 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

boutell avatar Oct 02 '19 13:10 boutell

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?

kingsleytorlowei avatar Oct 14 '19 06:10 kingsleytorlowei

Yes that would make sense. I think it was the case once.

On Mon, Oct 14, 2019 at 2:45 AM Kingsley Torlowei [email protected] 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

boutell avatar Oct 14 '19 11:10 boutell

(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 [email protected] wrote:

Yes that would make sense. I think it was the case once.

On Mon, Oct 14, 2019 at 2:45 AM Kingsley Torlowei < [email protected]> 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

boutell avatar Oct 14 '19 11:10 boutell

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?

kingsleytorlowei avatar Oct 14 '19 15:10 kingsleytorlowei

This was more about the sub-section "trash" rows. Are those fixed?

abea avatar Oct 14 '19 16:10 abea

Would something like this be appropriate? Screen Shot 2019-10-16 at 2 35 09 PM

kingsleytorlowei avatar Oct 16 '19 18:10 kingsleytorlowei

That looks much better!

abea avatar Oct 16 '19 20:10 abea

I currently have a pull request to fix this issue. #2089

kingsleytorlowei avatar Nov 21 '19 07:11 kingsleytorlowei

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)

abea avatar Jan 02 '20 15:01 abea

Hey can I work on this, I think, I may help in this.

themayank-goyal avatar Feb 04 '23 14:02 themayank-goyal

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.

BoDonkey avatar Feb 04 '23 15:02 BoDonkey

Where can i get the emails to ask for the clarification of this issue. I am a newbie in the opensource contribution.

Gaurav-create948 avatar Apr 14 '23 14:04 Gaurav-create948

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.

BoDonkey avatar Apr 14 '23 15:04 BoDonkey

can i start on this ?

yogeshsingh2672000 avatar May 01 '23 17:05 yogeshsingh2672000

@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

BoDonkey avatar May 01 '23 18:05 BoDonkey

can i start on this? i think i can resolve this

aditya162002 avatar Jul 03 '23 20:07 aditya162002

@aditya162002 Sure! Just read through the comments above first. Again, this is an Apostrophe 2 bug. Cheers, Bob

BoDonkey avatar Jul 04 '23 09:07 BoDonkey

I would like to give it a try. Can I start work on this?

shokerm avatar Aug 01 '23 12:08 shokerm

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

BoDonkey avatar Aug 01 '23 13:08 BoDonkey

hey @shokerm are you still working on this issue can i help in any way

Harshitahusts avatar Aug 21 '23 22:08 Harshitahusts

hi @Harshitahusts you are welcome to give it a try.

shokerm avatar Aug 22 '23 06:08 shokerm

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.

boutell avatar Aug 22 '23 13:08 boutell