Skip to content
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.

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

new 254 branch #3

Open
wants to merge 164 commits into
base: master
Choose a base branch
from
Open

new 254 branch #3

wants to merge 164 commits into from

Conversation

lizard-boy
Copy link

No description provided.

mgoldspink-salesforce and others added 30 commits September 22, 2021 13:36
W-10027913 - changed position of asterisk to right for components
On of the most common tasks all people do when customizing newport is disable the animated labels. Unfortunately it's not a straightforward task. So this PR is making it straight forward by adding a simple boolean config property to turn them on or off.

I need this for the OS Zen Garden css files because in the ones I'm creating them all need the animated labels turned off. So doing this saves a lot of time.

In addition, the way the webpack generation was happening, it was incredibly slow because it had to rebuild the entire JS React app if a css change was made due to the CSS being included in preview.js. So I've sped it up by adding some simple JS into the preview-head.html which listens for a rebuild command and updates the css that way. The additional benefit of this is that this css also includes SourceMaps so chrome and other browsers will now nicely link back to the source file for specific files!

Also tidied up commented out dead code.
A common initial change after removing the animated labels is to get back to bordered slds style input boxes. This is another config switch I've added to support this.

Since bordered inputs never make sense with the animated logo, it's controlled by the animated logo property too. So both have to be set to false for it to work.
For some reason the stories had an extra depth to it that it doesn't need in main.js

The omniscript.lwc.stories.js wasn't correctly detecting the changes and therefore missed the necessary class.
Also need to update version to 236 now.
… change and not cleaning itself up. This meant that it would slowly grind to a slow halt after a few changes.
This page talks about how to get started with the css zengarden demo. It covers how to checkout the branch, include the OS we rebranded as a datapack, the steps taken to rebrand it.

I've also fixed up styling of code samples. Also for some reason the docs had been removed from the previewer so added them back in.
@W-9802333 : Adding max-height and overflow-y to be consistent with slds
Leon changed the way step chart items are rendered in v236 so that the elements are now divs wrapped in an li

As a part of this PR I've cleaned up all the special attributes that LWC adds as it was causing the following erros to be reported in storybook:

```
[BABEL] Note: The code generator has deoptimised the styling of /Users/mgoldspink/development/newport-design-system/ui/components/omniscript/lwc/omniscript.lwc.stories.js as it exceeds the max of 500KB.
```

I also formatted the indentations so it's easier to read properly
…docs

W-9894880 - Adding docs page on css zen garden
…84_v236

W-9900484 - fixing up styling of step chart
…erlined_input_config

W-9894880 - config option to render bordered input
W-10046261 Shorten the stepchart labels if it's on a mobile device
W-9891061 => style changes for fix now button
…v236

W-10211997: Merge 232.6 to v236 after resolving conflicts
amansoni08 and others added 29 commits March 28, 2023 11:56
…20021_Biscayne_Accessibility

W-12620021: Biscayne accessibility
W-12577132 : Removing multiple scroll bars from config card
…75172_category_list_bug

W-13175172: Signature - 44553461 - Bell Canada - Catalog hierarchy is not rendered completely in the UI
changes for step-chart color, success and warning messages and Editblock color contrast (@W-13633873)
 Added colour contrast fix for Radio group, Type ahead and knowledge component (@W-13628944)
cc for cancel button border (@W-13212546)
@W-13119960 Updated README.md for branch name
@W-15551913: changed readonly radio button color to light grey color
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR introduces significant updates to the Newport Design System, focusing on improved development experience and expanded B2B functionality.

  • Updated Storybook configuration in .storybook/preview-head.html for real-time style updates during development
  • Added new OmniScript configuration OSZenGarden.json for demonstrating CSS styling capabilities
  • Expanded B2B component imports in ui/components/b2b/base/_index.scss, indicating new features
  • Introduced new input styling options in ui/_config.scss for more flexible form design
  • Updated build process in scripts/dist.js and scripts/gulp/styles.js to use the latest sass compiler

17 file(s) reviewed, 13 comment(s)
Edit PR Review Bot Settings

@@ -1,4 +1,5 @@
<link rel="stylesheet" href="node_modules/highlight.js/styles/github.css"/>
<link id="nds-styles" rel="stylesheet" href="/assets/styles/index.css"></link>
Copy link

Choose a reason for hiding this comment

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

style: Consider using a relative path for the stylesheet href to improve portability

@@ -24,3 +25,23 @@
/* THE ABOVE OVERRIDE IS PURELY FOR THE EXAMPLES IN STORYBOOK */
}
</style>
<script>
(function() {
const storybookEventSource = new EventSource(`${location.protocol}//${location.host}/__webpack_hmr`);
Copy link

Choose a reason for hiding this comment

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

logic: EventSource may not be supported in all browsers. Consider adding a fallback or feature detection

Comment on lines +37 to +39
setTimeout(() => {
newportStorybookStyles.href = `/assets/styles/index.css?${Date.now()}`;
}, 250);
Copy link

Choose a reason for hiding this comment

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

style: Using setTimeout may lead to race conditions. Consider using requestAnimationFrame for more reliable timing

Comment on lines +41 to +43
} catch (e) {
// ignore
}
Copy link

Choose a reason for hiding this comment

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

logic: Silently ignoring errors may hide important issues. Consider logging errors or implementing proper error handling

@@ -24,3 +25,23 @@
/* THE ABOVE OVERRIDE IS PURELY FOR THE EXAMPLES IN STORYBOOK */
}
</style>
<script>
Copy link

Choose a reason for hiding this comment

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

style: This script might benefit from being moved to a separate JavaScript file for better maintainability

/// IMPORTANT: To use this you must also disable $enable-animated-inputs
///
/// @type Boolean
$enable-simple-underlined-inputs: $enable-animated-inputs or true !default;
Copy link

Choose a reason for hiding this comment

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

logic: The default value $enable-animated-inputs or true will always evaluate to true, negating the importance of $enable-animated-inputs. Consider changing to not $enable-animated-inputs if these options should be mutually exclusive.

position: absolute;
width: 100%;
text-align: center;
top: 26rem;
Copy link

Choose a reason for hiding this comment

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

style: Consider using relative units or flexbox for responsive positioning instead of fixed rem values

width: 100%;
text-align: center;
top: 26rem;
color: #747474;
Copy link

Choose a reason for hiding this comment

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

style: Use a color variable instead of a hard-coded hex value for better maintainability

Comment on lines +30 to +31
left: 0;
transform: translateX(-50%);
Copy link

Choose a reason for hiding this comment

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

style: translateX(-50%) may cause misalignment. Consider using left: 50% instead for better positioning.

Comment on lines +40 to 46
.nds-b2b-attribute-count {
font-size: .875rem;
color: #1564bf;
font-weight: 400;
background: none;
border: none;
}
Copy link

Choose a reason for hiding this comment

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

style: Consider using SCSS variables for colors and font sizes to maintain consistency.

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.