Skip to content

Conversation

@alololox
Copy link
Contributor

@alololox alololox commented Sep 1, 2021

WHY are these changes introduced?

There are multiple locations within the new markets section of the admin application where we render a Page component without spacing between the title and subtitle props:

Screen Shot 2021-09-01 at 3 29 17 PM

WHAT is this pull request doing?

Adds a compactTitle prop that appends a new class, SubtitleCompact, which removes the top margin.

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

export function Playground() {
  return (
    <Page title="Title" subtitle="Subtitle" compactTitle>
      <p>Test text</p>
    </Page>
  );
}

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@alololox alololox force-pushed the page/tight-subtitle branch from e44e457 to a04450d Compare September 1, 2021 22:33
@alololox alololox self-assigned this Sep 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2021

size-limit report

Path Size
cjs 163.46 KB (+0.02% 🔺)
esm 96.19 KB (+0.04% 🔺)
esnext 143.09 KB (+0.05% 🔺)
css 34.52 KB (+0.02% 🔺)

@alololox alololox force-pushed the page/tight-subtitle branch from a04450d to 275b9dc Compare September 1, 2021 22:38
@alololox alololox marked this pull request as ready for review September 1, 2021 22:44
.SubTitle {
margin-top: spacing(tight);
color: var(--p-text-subdued);
&.SubtitleCompact {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link

@benkovy benkovy left a comment

Choose a reason for hiding this comment

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

Looks good to me

@alololox alololox merged commit cd0c812 into main Sep 2, 2021
@alololox alololox deleted the page/tight-subtitle branch September 2, 2021 15:00
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