Skip to content

Commit

Permalink
Improve _ShareType.scss (matrix-org#8737)
Browse files Browse the repository at this point in the history
* Specify the button style explicitly removing the dependency on the mixin

The reset mixin can cause style inconsistencies by disrupting cascading arbitrarily, increasing the number of specified declarations more than needed. Though it might be useful for development, it is not necessary to use it, makes it difficult to grasp the style structure, and can be removed to optimize the structure.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Remove element='button' from AccessibleButton

Since AccessibleButton has role='button' by default, setting the element button property is redundant.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Protect mx_ShareType_option from being regressed structurally

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* yarn run lint:style --fix

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Wrap buttons with declarations for spacing

box-sizing is not required for the buttons or the wrapper.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* yarn run lint:style --fix

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* fix eslint errors

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Fix LocationShareMenu-test.tsx

AccessibleButton is div by default

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Reflect the review

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Revert "Remove element='button' from AccessibleButton"

This reverts commit af78d27.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Revert "Fix LocationShareMenu-test.tsx"

This reverts commit 7d783a7.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
  • Loading branch information
luixxiul authored and JanBurp committed Jun 14, 2022
1 parent 3ef6c4f commit d5b1374
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 31 deletions.
51 changes: 29 additions & 22 deletions res/css/components/views/location/_ShareType.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,35 @@ limitations under the License.
padding: 60px $spacing-12 $spacing-32;

color: $primary-content;

.mx_ShareType_wrapper_options {
display: flex;
flex-direction: column;
row-gap: $spacing-12;
width: 100%;
margin-top: $spacing-12;

.mx_ShareType_option {
display: flex;
align-items: center;
justify-content: flex-start;
padding: $spacing-8 $spacing-20;
background: none;

border: 1px solid $quinary-content;
border-radius: 8px;

font-size: $font-15px;
font-family: inherit;
line-height: inherit;
color: $primary-content;

&:hover,
&:focus {
border-color: $accent;
}
}
}
}

.mx_ShareType_badge {
Expand All @@ -43,28 +72,6 @@ limitations under the License.
text-align: center;
}

.mx_ShareType_option {
@mixin ButtonResetDefault;

width: 100%;
display: flex;
flex-direction: row;
align-items: center;
justify-content: flex-start;
padding: $spacing-8 $spacing-20;
margin-top: $spacing-12;

color: $primary-content;
border: 1px solid $quinary-content;
border-radius: 8px;

font-size: $font-15px;

&:hover, &:focus {
border-color: $accent;
}
}

.mx_ShareType_option-icon {
height: 40px;
width: 40px;
Expand Down
20 changes: 11 additions & 9 deletions src/components/views/location/ShareType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,17 @@ const ShareType: React.FC<Props> = ({
return <div className='mx_ShareType'>
<LocationIcon className='mx_ShareType_badge' />
<Heading className='mx_ShareType_heading' size='h3'>{ _t("What location type do you want to share?") }</Heading>
{ enabledShareTypes.map((type) =>
<ShareTypeOption
key={type}
onClick={() => setShareType(type)}
label={labels[type]}
shareType={type}
data-test-id={`share-location-option-${type}`}
/>,
) }
<div className='mx_ShareType_wrapper_options'>
{ enabledShareTypes.map((type) =>
<ShareTypeOption
key={type}
onClick={() => setShareType(type)}
label={labels[type]}
shareType={type}
data-test-id={`share-location-option-${type}`}
/>,
) }
</div>
</div>;
};

Expand Down

0 comments on commit d5b1374

Please sign in to comment.