Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add persistence of collapsed state of space panel #9245

Closed
wants to merge 12 commits into from

Conversation

gefgu
Copy link
Contributor

@gefgu gefgu commented Sep 5, 2022

Signed-off-by: gefgu [email protected]

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Notes: Add persistence of collapsed state of space panel. It fixes element-hq/element-web#23172

Type: enhancement

Behavior: (Hard reload was made by ctrl + shift + r)

space-panel-demonstration_2xspeed.mp4

This change was made using LocalStorage implemented inside of the component. I'm not sure it's the best solution because of most of the other LocalStorage values are implemented using stores. In this way, I want feedback about my approach and I'm willing to keep working on this issue until it is resolved.


Here's what your changelog entry will look like:

✨ Features

@gefgu gefgu requested a review from a team as a code owner September 5, 2022 19:31
@github-actions github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Sep 5, 2022
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! It looks pretty straightforward, though I think a simple cypress test or similar would be a great addition to this.

src/components/views/spaces/SpacePanel.tsx Outdated Show resolved Hide resolved
@gefgu gefgu force-pushed the persist-minimised-space-panel branch from 3f5290c to 50f1a55 Compare September 8, 2022 19:06
@gefgu
Copy link
Contributor Author

gefgu commented Sep 8, 2022

I've added the cypress test and would be delighted if you could review it - It is the first time I work with cypress :). Also, I've removed the try/catch block. It does not seems to be necessary.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally this looks good to me - thanks!

I've merged develop in hopes of kicking the CI into working properly - apologies if this interferes with your workflow.

cypress/e2e/space-panel/space-panel.spec.ts Outdated Show resolved Hide resolved
src/components/views/spaces/SpacePanel.tsx Outdated Show resolved Hide resolved
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still looks good to me overall - the CI is very unhappy for some reason though.

cy.get(".mx_SpacePanel_toggleCollapse").click();
cy.get(".mx_SpacePanel").should("not.have.class", "collapsed");
cy.reload(true);
cy.get(".mx_SpacePanel").should("not.have.class", "collapsed");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also probably check to see if clicking it again and reloading causes it to un-collapse

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App should remember whether the space panel was expanded
3 participants