Skip to content

Make Dialog positioning more robust #1404

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.

Sign up for GitHub

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

Merged
merged 33 commits into from
Aug 3, 2018
Merged

Make Dialog positioning more robust #1404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
717a564
WIP
vintik May 7, 2018
3e965b8
WIP - building more test cases and trying to understand how to fix po…
vintik May 8, 2018
379be5b
trying to consolidate all dialogs to have the offset margin hardcode
vintik May 10, 2018
2761193
WIP
vintik May 10, 2018
36a9c7f
starting to finally understand things
vintik May 11, 2018
caabdc4
some good progress
vintik May 11, 2018
d46a51f
get margins working somehow
vintik May 29, 2018
4e7561c
WIP
vintik May 30, 2018
19e5cd1
get popovers working
vintik Jun 1, 2018
613dd87
WIP
vintik Jun 1, 2018
2630c25
get combobox working
vintik Jun 1, 2018
ce6383d
some cleanup and return unreadable formatting
vintik Jun 4, 2018
8e155d4
WIP
vintik Jun 18, 2018
bd9da67
what am i doing?
vintik Jun 19, 2018
2ff0d04
Need to understand how focusing works and decouple
vintik Jun 21, 2018
43cf9de
Got popovers working. trap focus needs to be tested
vintik Jul 2, 2018
64b1e44
gotta figure out all this inversing on dropdowns
vintik Jul 2, 2018
a4a712d
WIP - getting weird position consistency
vintik Jul 3, 2018
599785d
linter is stupid, but managed to 'autofix' a bunch of stuff luckily
vintik Jul 3, 2018
c80479f
tests suck
vintik Jul 4, 2018
25e693e
Fix stuff
vintik Jul 4, 2018
af6f7eb
i'll never understand the linter because it wont tell me whats wrong
vintik Jul 4, 2018
ba7a82d
update snapshots? why do they all update?
vintik Jul 4, 2018
593efcb
Remove custom storybook of Victors
vintik Jul 5, 2018
039a8ac
some cleanup
vintik Jul 5, 2018
da24687
Support flipping of dialogs
vintik Jul 25, 2018
efda782
Merge branch 'master' into vintik/dialogs
vintik Jul 25, 2018
3e943b9
Add a first version of release notes for dialogs changes
vintik Jul 25, 2018
f49b858
autofix prettier issues
vintik Jul 25, 2018
a75ee19
Deprecate offset prop for dialog components
vintik Jul 25, 2018
2c9d891
Cleanup release notes
vintik Jul 25, 2018
e6e2344
More lint issues and potentially fixes the voiceover tooltip issue
vintik Jul 25, 2018
4b78eb3
more lint fix
vintik Jul 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions RELEASENOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@ These are changes that have backwards-compatible solutions present and that comp

### Latest Release

## Release 0.8.21

**Bugfixes**

* Positioning behavior of Dialog components that use nubbins has changed. This applies to `Popover`, `Tooltip`, `Datepicker`, `Dropdown`. Previously the nubbin would be misaligned due to hardcoded margins that would get added onto the dialog component. It will now instead calculate the offsets and include them in the positioning logic and add/subtract from the left and top.

* Dialogs that use nubbins would previously have the nubbins point at the location on the reference trigger component (i.e. `Button`). Details:
* If a Popover had an align of `top left`, that meant the nubbin would point at the top left hand side of the `Button`.
* The behavior now is that the nubbin will always position the Dialog element as needed to ensure it points at the center of the desired side.
* In the case of a `top left` align the left will only designate the location of the nubbin on the Dialog.
* We may decide to bring back the ability to control both the nubbin location on the Dialog, but also the location at which it points to on the reference element. Much of the logic surrounding nubbins has been broken at the seams and edge cases for a long time. This change has been done in order to provide a more robust and dependable solution.
* Any dialog that uses an `offset` prop will need to be manually readjusted.
* Deprecate `offset` prop for `Dropdown` and `Popover`. The manual setting of positional offset of dialog components has been deemed unreliable. Position logic has been re-written to deliver better and more reliable positioning. Please create an issue if you have an edge case not covered by the built-in logic.
* In the future we may change the props on some of these Dialog components to ensure a more consistent positioning API with hopefully some less head-scratching in terms of how to use it. This may include deprecating certain props and introduction of other props.


## Release 0.8.20

**Bugfixes**
Expand Down
14 changes: 7 additions & 7 deletions components/combobox/__tests__/__snapshots__/combobox.snapshot-test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ exports[`Base Custom Menu Item Open HTML Snapshot 1`] = `
<div class=\\"slds-combobox slds-dropdown-trigger slds-dropdown-trigger_click ignore-react-onclickoutside slds-is-open\\" aria-expanded=\\"true\\" aria-haspopup=\\"listbox\\" aria-owns=\\"combobox-unique-id-listbox\\" role=\\"combobox\\">
<div class=\\"slds-combobox__form-element slds-input-has-icon slds-input-has-icon_right\\" role=\\"none\\"><input type=\\"text\\" aria-autocomplete=\\"list\\" aria-controls=\\"combobox-unique-id-listbox\\" autocomplete=\\"off\\" class=\\"slds-input slds-combobox__input\\" id=\\"combobox-unique-id\\" placeholder=\\"Search Salesforce\\" role=\\"textbox\\" value=\\"\\" /><svg aria-hidden=\\"true\\"
class=\\"slds-input__icon slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#search\\"></use></svg></div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid\\" role=\\"presentation\\">
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-1\\" class=\\"slds-media slds-listbox__option slds-listbox__option_entity slds-listbox__option_has-meta\\" role=\\"option\\"><span><span class=\\"slds-listbox__option-text slds-listbox__option-text_entity\\">Acme</span>
<span
Expand Down Expand Up @@ -627,7 +627,7 @@ exports[`Base Open Custom Class Name HTML Snapshot 1`] = `
<div class=\\"slds-combobox slds-dropdown-trigger slds-dropdown-trigger_click ignore-react-onclickoutside slds-is-open this-is-the-input\\" aria-expanded=\\"true\\" aria-haspopup=\\"listbox\\" aria-owns=\\"combobox-unique-id-listbox\\" role=\\"combobox\\">
<div class=\\"slds-combobox__form-element slds-input-has-icon slds-input-has-icon_right\\" role=\\"none\\"><input type=\\"text\\" aria-autocomplete=\\"list\\" aria-controls=\\"combobox-unique-id-listbox\\" autocomplete=\\"off\\" class=\\"slds-input slds-combobox__input\\" id=\\"combobox-unique-id\\" placeholder=\\"Search Salesforce\\" role=\\"textbox\\" value=\\"\\" /><svg aria-hidden=\\"true\\"
class=\\"slds-input__icon slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#search\\"></use></svg></div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid this-is-the-menu\\" role=\\"presentation\\">
<li class=\\"slds-dropdown__header slds-truncate\\" title=\\"Accounts\\" role=\\"separator\\"><span class=\\"slds-text-title_caps this-is-the-menu-sub-header\\">Accounts</span></li>
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-1\\" class=\\"slds-media slds-listbox__option slds-listbox__option_entity slds-listbox__option_has-meta\\" role=\\"option\\"><span class=\\"slds-media__figure\\"><span class=\\"slds-icon_container slds-icon-standard-account\\"><svg aria-hidden=\\"true\\" class=\\"slds-icon\\"><use xlink:href=\\"/assets/icons/standard-sprite/svg/symbols.svg#account\\"></use></svg><span class=\\"slds-assistive-text\\">Account</span></span>
Expand Down Expand Up @@ -823,7 +823,7 @@ exports[`Base Open HTML Snapshot 1`] = `
<div class=\\"slds-combobox slds-dropdown-trigger slds-dropdown-trigger_click ignore-react-onclickoutside slds-is-open\\" aria-expanded=\\"true\\" aria-haspopup=\\"listbox\\" aria-owns=\\"combobox-unique-id-listbox\\" role=\\"combobox\\">
<div class=\\"slds-combobox__form-element slds-input-has-icon slds-input-has-icon_right\\" role=\\"none\\"><input type=\\"text\\" aria-autocomplete=\\"list\\" aria-controls=\\"combobox-unique-id-listbox\\" autocomplete=\\"off\\" class=\\"slds-input slds-combobox__input\\" id=\\"combobox-unique-id\\" placeholder=\\"Search Salesforce\\" role=\\"textbox\\" value=\\"\\" /><svg aria-hidden=\\"true\\"
class=\\"slds-input__icon slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#search\\"></use></svg></div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid\\" role=\\"presentation\\">
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-1\\" class=\\"slds-media slds-listbox__option slds-listbox__option_entity slds-listbox__option_has-meta\\" role=\\"option\\"><span class=\\"slds-media__figure\\"><span class=\\"slds-icon_container slds-icon-standard-account\\"><svg aria-hidden=\\"true\\" class=\\"slds-icon\\"><use xlink:href=\\"/assets/icons/standard-sprite/svg/symbols.svg#account\\"></use></svg><span class=\\"slds-assistive-text\\">Account</span></span>
</span><span class=\\"slds-media__body\\"><span class=\\"slds-listbox__option-text slds-listbox__option-text_entity\\">Acme</span><span class=\\"slds-listbox__option-meta slds-listbox__option-meta_entity\\">Account • San Francisco</span></span>
Expand Down Expand Up @@ -960,7 +960,7 @@ exports[`Base Open Menu Inherit Width Of Menu HTML Snapshot 1`] = `
<div class=\\"slds-combobox slds-dropdown-trigger slds-dropdown-trigger_click ignore-react-onclickoutside slds-is-open\\" aria-expanded=\\"true\\" aria-haspopup=\\"listbox\\" aria-owns=\\"combobox-unique-id-listbox\\" role=\\"combobox\\">
<div class=\\"slds-combobox__form-element slds-input-has-icon slds-input-has-icon_right\\" role=\\"none\\"><input type=\\"text\\" aria-autocomplete=\\"list\\" aria-controls=\\"combobox-unique-id-listbox\\" autocomplete=\\"off\\" class=\\"slds-input slds-combobox__input\\" id=\\"combobox-unique-id\\" placeholder=\\"Search Salesforce\\" role=\\"textbox\\" value=\\"\\" /><svg aria-hidden=\\"true\\"
class=\\"slds-input__icon slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#search\\"></use></svg></div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid\\" role=\\"presentation\\" style=\\"width:auto;max-width:500px;\\">
<li class=\\"slds-dropdown__header slds-truncate\\" title=\\"Accounts\\" role=\\"separator\\"><span class=\\"slds-text-title_caps\\">Accounts</span></li>
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-2\\" class=\\"slds-media slds-listbox__option slds-listbox__option_entity slds-listbox__option_has-meta\\" role=\\"option\\"><span class=\\"slds-media__body\\"><span class=\\"slds-listbox__option-text slds-listbox__option-text_entity\\">Salesforce.com, Inc.</span>
Expand Down Expand Up @@ -1096,7 +1096,7 @@ exports[`Base Open Menu Sub Header HTML Snapshot 1`] = `
<div class=\\"slds-combobox slds-dropdown-trigger slds-dropdown-trigger_click ignore-react-onclickoutside slds-is-open\\" aria-expanded=\\"true\\" aria-haspopup=\\"listbox\\" aria-owns=\\"combobox-unique-id-listbox\\" role=\\"combobox\\">
<div class=\\"slds-combobox__form-element slds-input-has-icon slds-input-has-icon_right\\" role=\\"none\\"><input type=\\"text\\" aria-autocomplete=\\"list\\" aria-controls=\\"combobox-unique-id-listbox\\" autocomplete=\\"off\\" class=\\"slds-input slds-combobox__input\\" id=\\"combobox-unique-id\\" placeholder=\\"Search Salesforce\\" role=\\"textbox\\" value=\\"\\" /><svg aria-hidden=\\"true\\"
class=\\"slds-input__icon slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#search\\"></use></svg></div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid\\" role=\\"presentation\\">
<li class=\\"slds-dropdown__header slds-truncate\\" title=\\"Accounts\\" role=\\"separator\\"><span class=\\"slds-text-title_caps\\">Accounts</span></li>
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-2\\" class=\\"slds-media slds-listbox__option slds-listbox__option_entity slds-listbox__option_has-meta\\" role=\\"option\\"><span class=\\"slds-media__body\\"><span class=\\"slds-listbox__option-text slds-listbox__option-text_entity\\">Salesforce.com, Inc.</span>
Expand Down Expand Up @@ -2742,7 +2742,7 @@ exports[`Readonly Single Selection Custom Menu Item Open HTML Snapshot 1`] = `
<span
class=\\"slds-icon_container slds-input__icon slds-input__icon_right\\"><svg aria-hidden=\\"true\\" class=\\"slds-icon slds-icon_x-small slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#down\\"></use></svg></span>
</div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid slds-dropdown_length-with-icon-5\\" role=\\"presentation\\">
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"false\\" id=\\"combobox-unique-id-listbox-option-1\\" class=\\"slds-media slds-listbox__option slds-listbox__option_plain slds-media_small slds-media_center\\" role=\\"option\\"><span class=\\"slds-media__figure\\"><span><svg aria-hidden=\\"true\\" class=\\"slds-listbox__icon-selected slds-icon slds-icon--x-small slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#check\\"></use></svg></span></span>
<span
Expand Down Expand Up @@ -3389,7 +3389,7 @@ exports[`Readonly Single Selection Selected Open HTML Snapshot 1`] = `
<span
class=\\"slds-icon_container slds-input__icon slds-input__icon_right\\"><svg aria-hidden=\\"true\\" class=\\"slds-icon slds-icon_x-small slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#down\\"></use></svg></span>
</div>
<div id=\\"combobox-unique-id-listbox\\" role=\\"listbox\\">
<div role=\\"listbox\\" id=\\"combobox-unique-id-listbox\\">
<ul class=\\"slds-listbox slds-listbox_vertical slds-dropdown slds-dropdown_fluid slds-dropdown_length-with-icon-5\\" role=\\"presentation\\">
<li class=\\"slds-listbox__item\\" role=\\"presentation\\"><span aria-selected=\\"true\\" id=\\"combobox-unique-id-listbox-option-1\\" class=\\"slds-media slds-listbox__option slds-listbox__option_plain slds-media_small slds-media_center slds-is-selected\\" role=\\"option\\"><span class=\\"slds-media__figure\\"><span><svg aria-hidden=\\"true\\" class=\\"slds-listbox__icon-selected slds-icon slds-icon--x-small slds-icon-text-default\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#check\\"></use></svg></span></span>
<span
Expand Down
4 changes: 2 additions & 2 deletions components/filter/__tests__/__snapshots__/filter.snapshot-test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ exports[`AssistiveText Filter 1`] = `

exports[`AssistiveText Filter HTML Snapshot 1`] = `
"<div class=\\"slds-filters__item slds-grid slds-grid--vertical-align-center\\">
<div class=\\"slds-grow\\" style=\\"display:inline;\\"><button class=\\"slds-button--reset slds-grow slds-has-blur-focus\\" id=\\"sample-panel-filtering-show-me\\" tabindex=\\"0\\"><span class=\\"slds-assistive-text\\">Edit filter:</span><p class=\\"slds-text-body--small\\">Show Me</p><p>All Products</p></button></div>
<div class=\\"slds-grow\\" style=\\"display:inline-block;\\"><button class=\\"slds-button--reset slds-grow slds-has-blur-focus\\" id=\\"sample-panel-filtering-show-me\\" tabindex=\\"0\\"><span class=\\"slds-assistive-text\\">Edit filter:</span><p class=\\"slds-text-body--small\\">Show Me</p><p>All Products</p></button></div>
<button
class=\\"slds-button slds-button--icon slds-button--icon-bare slds-button--icon-small\\" title=\\"Remove Filter: Show Me All Products\\" type=\\"button\\"><svg aria-hidden=\\"true\\" class=\\"slds-button__icon slds-button__icon--hint\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#close\\"></use></svg><span class=\\"slds-assistive-text\\">Remove Filter: Show Me All Products</span></button>
</div>"
Expand Down Expand Up @@ -222,7 +222,7 @@ exports[`Filter Base Snapshot 1`] = `

exports[`Filter Base with custom className Snapshot 1`] = `
"<div class=\\"slds-filters__item slds-grid slds-grid--vertical-align-center MY_CUSTOM_CLASS_NAME\\">
<div class=\\"slds-grow\\" style=\\"display:inline;\\"><button class=\\"slds-button--reset slds-grow slds-has-blur-focus\\" id=\\"sample-panel-filtering-show-me\\" tabindex=\\"0\\"><span class=\\"slds-assistive-text\\">Edit filter:</span><p class=\\"slds-text-body--small\\">Show Me</p><p>All Products</p></button></div>
<div class=\\"slds-grow\\" style=\\"display:inline-block;\\"><button class=\\"slds-button--reset slds-grow slds-has-blur-focus\\" id=\\"sample-panel-filtering-show-me\\" tabindex=\\"0\\"><span class=\\"slds-assistive-text\\">Edit filter:</span><p class=\\"slds-text-body--small\\">Show Me</p><p>All Products</p></button></div>
<button
class=\\"slds-button slds-button--icon slds-button--icon-bare slds-button--icon-small\\" title=\\"Remove Filter: Show Me All Products\\" type=\\"button\\"><svg aria-hidden=\\"true\\" class=\\"slds-button__icon slds-button__icon--hint\\"><use xlink:href=\\"/assets/icons/utility-sprite/svg/symbols.svg#close\\"></use></svg><span class=\\"slds-assistive-text\\">Remove Filter: Show Me All Products</span></button>
</div>"
Expand Down
8 changes: 8 additions & 0 deletions components/menu-dropdown/check-props.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ if (process.env.NODE_ENV !== 'production') {
'Please use isOpen instead. It provides a consistent prop that aligns with other componenents.'
);

deprecatedProperty(
COMPONENT,
props.offset,
'offset',
undefined,
'The manual setting of positional offset of dialog components has been deemed unreliable. Position logic has been re-written to deliver better and more reliable positioning. Please create an issue if you have an edge case not covered by the built-in logic.'
);

oneOfRequiredProperty(COMPONENT, {
options: props.options,
children: props.children,
Expand Down
Loading