Remove old user and room settings #2554
Conversation
|
conclusion: this PR is probably not going to land as-is before FOSDEM. Instead, we're going to enable the new settings for people and take off the temporary tab, leaving removal of the old settings for later. |
turt2live
force-pushed
the
travis/settings/delete-the-old
branch
from
f562799 to
397e36e
Compare
This takes out the old user and room settings, replacing the paths with the new dialog editions. The labs setting has been removed in order to support this change. In addition to removing the old components outright, some older components which were only used by the settings pages have been removed. The exception is the ColorSettings component as it has a high chance of sticking around in the future. Styles that were shared by the settings components have been broken out to dedicated sections, making it easier to remove the old styles entirely. Some stability testing of the app has been performed to ensure the app still works, however given the scope of this change there is a possibility of some broken functionality.
turt2live
force-pushed
the
travis/settings/delete-the-old
branch
from
397e36e to
eac50aa
Compare
| @@ -0,0 +1,19 @@ | |||
| /* | |||
| Copyright 2019 New Vector Ltd. | |||
There was a problem hiding this comment.
Ultra nit: It's currently more common to write Ltd without a full stop in these headers
There was a problem hiding this comment.
in all the legal documents it's with a full stop, so I'm tempted to say we've been doing it wrong elsewhere.
There was a problem hiding this comment.
Haha, perhaps so... My OCD just hopes we'll choose one way or the other... 😰
There was a problem hiding this comment.
What if we just make it 50/50 exactly? Then you get the best of both worlds and a pleasing number.
| /* | ||
| * TODO: Make things use this. This is all WIP - see UserSettings.js for usage. | ||
| */ | ||
| // TODO: Decommission. |
There was a problem hiding this comment.
I think it would be nice to file an issue and link it here for this future work.
| @@ -77,7 +77,7 @@ export default React.createClass({ | |||
| <div role="alert">{ this.state.authError.message || this.state.authError.toString() }</div> | |||
| <br /> | |||
| <AccessibleButton onClick={this._onDismissClick} | |||
| className="mx_UserSettings_button" | |||
| className="mx_GeneralButton" | |||
There was a problem hiding this comment.
What is a GeneralButton? Could it just be mx_Button?
There was a problem hiding this comment.
A GeneralButton is a button that was using the user settings classes and I didn't want to break anything, so included the extra styles. I haven't tested, but I have a burning feeling that trying to make it a Button will be more effort than it's worth for this PR.
There was a problem hiding this comment.
Okay, fair enough. Maybe make a future issue to clean up / simplify button classes?
|
I've created a bunch of issues for the concerns btw. See 04f2375 |
Part of element-hq/element-web#8207
This might be a change which we want to land after FOSDEM.We decided to do this, and here we are.This takes out the old user and room settings, replacing the paths with the new dialog editions. The labs setting has been removed in order to support this change.
In addition to removing the old components outright, some older components which were only used by the settings pages have been removed. The exception is the ColorSettings component as it has a high chance of sticking around in the future.
Styles that were shared by the settings components have been broken out to dedicated sections, making it easier to remove the old styles entirely.
Some stability testing of the app has been performed to ensure the app still works, however given the scope of this change there is a possibility of some broken functionality.