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

Range control marks center aligned #35206

Conversation

HILAYTRIVEDI
Copy link
Contributor

@HILAYTRIVEDI HILAYTRIVEDI commented Sep 29, 2021

Description

The top parameter of the marks stylings is set to top: -7px, due to which it will align the marks to the center of the track. This is tested in the storybook.

##Screenshot
https://www.awesomescreenshot.com/image/14183951?key=b3ec1c94ec68ebfb5c10903b915778b4

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 29, 2021
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @HILAYTRIVEDI! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

Copy link
Contributor

@stokesman stokesman 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 having a go at this one @HILAYTRIVEDI. Here's what I see (Testing on Chromium and macOS):
image

The marks are even farther out place there. Looks like we have to get the placement consistent in all browsers before we know what top should be.

I've tried Chrome, Firefox and Edge on Windows 10 and they all render the marks above the rails. I would be helpful to know what browser and OS you are testing with.

@HILAYTRIVEDI
Copy link
Contributor Author

Thanks for having a go at this one @HILAYTRIVEDI. Here's what I see (Testing on Chromium and macOS): image

The marks are even farther out place there. Looks like we have to get the placement consistent in all browsers before we know what top should be.

I've tried Chrome, Firefox and Edge on Windows 10 and they all render the marks above the rails. I would be helpful to know what browser and OS you are testing with.

Can you please describe the steps to recreate them?

@stokesman
Copy link
Contributor

Can you please describe the steps to recreate them?

To test this I visited http://gutenberg.run/35206 and executed the following script in the console:

( function ( wp ) {
    const registerPlugin = wp.plugins.registerPlugin;
    const PluginSidebar = wp.editPost.PluginSidebar;
    const { createElement:el } = wp.element;
    const { RangeControl, } = wp.components;

    registerPlugin( 'marked-range-controls', {
        render: function () {
            return el(
                PluginSidebar,
                {
                    name: 'marked-range-controls',
                    icon: 'admin-post',
                    title: 'Marked RangeControls',
                },
                el( 'style', null, `
                    style + .components-range-control{
                        margin-top: 2em;
                    }
                    .components-range-control{
                        padding: .5em 1em;
                    }
                `),
                el( RangeControl, { onChange: v => console.log(v), min: 0, max: 100, step: 10, marks: true }),
                el( RangeControl, { onChange: v => console.log(v), min: 0, max: 100, step: 1,
                    marks: [ { label: 'min', value:0 }, { label: '¡', value: 50 }, { label: 'max', value:100 }]
                } ),
            );
        },
    } );
} )( window.wp );

That adds a sidebar accessible in the primary toolbar (see the focused button with the pushpin in the screenshot),
image
the sidebar contains the range controls.

The browsers/OS combos I tested with were mentioned in my last comment.

I haven't found where it renders like your screenshot (though I did come across another screenshot in #33824 (review) that matched the one you provided in the issue).

I would like to be able to reproduce that too. What browser and OS combination are you using to test?

@HILAYTRIVEDI
Copy link
Contributor Author

Can you please describe the steps to recreate them?

To test this I visited http://gutenberg.run/35206 and executed the following script in the console:

( function ( wp ) {
    const registerPlugin = wp.plugins.registerPlugin;
    const PluginSidebar = wp.editPost.PluginSidebar;
    const { createElement:el } = wp.element;
    const { RangeControl, } = wp.components;

    registerPlugin( 'marked-range-controls', {
        render: function () {
            return el(
                PluginSidebar,
                {
                    name: 'marked-range-controls',
                    icon: 'admin-post',
                    title: 'Marked RangeControls',
                },
                el( 'style', null, `
                    style + .components-range-control{
                        margin-top: 2em;
                    }
                    .components-range-control{
                        padding: .5em 1em;
                    }
                `),
                el( RangeControl, { onChange: v => console.log(v), min: 0, max: 100, step: 10, marks: true }),
                el( RangeControl, { onChange: v => console.log(v), min: 0, max: 100, step: 1,
                    marks: [ { label: 'min', value:0 }, { label: '¡', value: 50 }, { label: 'max', value:100 }]
                } ),
            );
        },
    } );
} )( window.wp );

That adds a sidebar accessible in the primary toolbar (see the focused button with the pushpin in the screenshot), image the sidebar contains the range controls.

The browsers/OS combos I tested with were mentioned in my last comment.

I haven't found where it renders like your screenshot (though I did come across another screenshot in #33824 (review) that matched the one you provided in the issue).

I would like to be able to reproduce that too. What browser and OS combination are you using to test?

@stokesman Its chromium and we are getting this results in that.

@ciampo ciampo self-requested a review October 1, 2021 12:21
@ciampo
Copy link
Contributor

ciampo commented Aug 15, 2024

Closing as duplicate of #64487 (that got merged in the meantime)

@ciampo ciampo closed this Aug 15, 2024
Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: HILAYTRIVEDI <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: ciampo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants