Skip to content

Conversation

dleroux
Copy link
Contributor

@dleroux dleroux commented Nov 8, 2019

WHY are these changes introduced?

There's an issue in web where the content of the TopBar has not loaded when the StickyManager gets instantiated. The height of the bar is 0, therefore, the elements that should 'stick' below the top bar don't.

WHAT is this pull request doing?

The height of the bar is set on the TopBar, but not on the wrapper in the Frame where the querySelector happens. This change adds the same height on the wrapper.

Sticking

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2019

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified2
Files potentially affected2

Details

All files potentially affected (total: 2)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/Frame/Frame.scss (total: 2)

Files potentially affected (total: 2)


This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

@dleroux dleroux force-pushed the sticky-topbar-height branch from 7e0c48d to 6325bcd Compare November 8, 2019 18:41
@dleroux dleroux changed the title [WIP] [Frame][TopBar] Add height to TopBar [Frame TopBar] Add height to TopBar Nov 8, 2019
@dleroux dleroux requested a review from alex-page November 11, 2019 12:48
Copy link
Contributor

@dfmcphee dfmcphee left a comment

Choose a reason for hiding this comment

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

The change makes sense to me. I 🎩ed in web and it fixed the problem I was seeing. 👍

Copy link
Member

@AndrewMusgrave AndrewMusgrave left a comment

Choose a reason for hiding this comment

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

Works like a charm ✅

@dleroux dleroux force-pushed the sticky-topbar-height branch from 6325bcd to 4da3e85 Compare November 21, 2019 14:39
Co-Authored-By: Andrew Musgrave <andrew.musgrave@shopify.com>
@dleroux dleroux merged commit 5c454fe into master Nov 21, 2019
@dleroux dleroux deleted the sticky-topbar-height branch November 21, 2019 14:52
@dleroux dleroux temporarily deployed to production December 4, 2019 14:42 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants