Skip to content

IAE-23322 Results list updates#69

Merged
davereed merged 13 commits into
masterfrom
Results-List-Updates
Sep 27, 2019
Merged

IAE-23322 Results list updates#69
davereed merged 13 commits into
masterfrom
Results-List-Updates

Conversation

@davereed
Copy link
Copy Markdown
Contributor

No description provided.

@@ -1,3 +1,8 @@
.result-item-wrapper {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated naming

height: 2.1rem;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you move tag related css to src/stylesheets/elements/_tags.scss

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved the display related classes out of inputs

@@ -1,26 +1,24 @@
.sds-tag--info-purple {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

colors should be added by using the color() function example color: color('primary');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to mixins

// Layouts
// -------------------------------------
@import '../layouts/subheader';
@import '../layouts/page';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

adding this could be misunderstood/misuse by some developers. Developers could think that page specific styles could be added here - All css should be broken down in to components and add it to their corresponding folder

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved the sds-page css to a component file, hopefully this is more clear.

@@ -42,6 +42,7 @@ SDS THEME CUSTOM STYLES
// Layouts
// -------------------------------------
@import '../layouts/subheader';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll be removing subheader as this seems to be causing confusion about the structure of the css

Comment thread src/stylesheets/layouts/_page.scss Outdated
@@ -0,0 +1,44 @@
sds-page {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

most of these styles are border related css - I think it will make sense to create a borders file. After moving the border styles from this file. The rest of the styles could be achieved by utilities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved border and sizing out to their own files

}

div[class^="grid-col-"] .sds-tag--static, div[class*=" grid-col-"] .sds-tag--static {
div[class^="grid-col-"] .sds-tag--static,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will affect all the tags inside a grid-col not only the tags located in the searchlist component

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing else was using this class, but I renamed and moved this to tags and created a block style modifier rather than calling it static.

*/ No newline at end of file
*/

$theme-header-min-width: 'desktop' !default;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This variable is for the header component - could you remove it from this PR

@@ -1,3 +1,8 @@
.result-item {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

components should start with 'sds' prefix

@davereed davereed merged commit 049e824 into master Sep 27, 2019
@davereed davereed deleted the Results-List-Updates branch October 29, 2019 15:36
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.

2 participants