Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Site Settings: move primary button to <SectionHeader>. #373

Merged
merged 1 commit into from
Nov 25, 2015

Conversation

mtias
Copy link
Member

@mtias mtias commented Nov 21, 2015

And update it to use <Button>. Part of series of iterative improvements in the site settings screens.

image

@mtias mtias added [Feature] Site Settings All other general site settings. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] In Progress labels Nov 21, 2015
@artpi
Copy link
Contributor

artpi commented Nov 24, 2015

Tested, Works.
I like that all the buttons change state.

I am kinda worried that this save button is in the same place as 'close' on Windows window.
People may associate that location with discarding changes...
Would be good to discuss it a bit

@mtias
Copy link
Member Author

mtias commented Nov 25, 2015

The section-header buttons is a pattern that will be extended to more sections as they adopt it. It's currently used in /plugins.

@artpi
Copy link
Contributor

artpi commented Nov 25, 2015

🚢

@artpi artpi added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 25, 2015
mtias added a commit that referenced this pull request Nov 25, 2015
Site Settings: move primary button to <SectionHeader>.
@mtias mtias merged commit 2d23fc3 into master Nov 25, 2015
onClick={ this.submitForm }
primary={ true }
type="submit"
disabled={ this.state.fetchingSettings || this.state.submittingForm }>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for the next time maybe you could close the Component in the same TAB where it was opened.

    <Button
        compact={ true }
        onClick={ this.submitForm }
        primary={ true }
        type="submit"
        disabled={ this.state.fetchingSettings || this.state.submittingForm }
    >
        { this.state.submittingForm
            ? this.translate( 'Saving…' )
// ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants