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

DS-799 Object Inline List deprecation #2515

Merged
merged 8 commits into from Jul 28, 2022

Conversation

MarcinMr
Copy link
Collaborator

Jira

https://pegadigitalit.atlassian.net/browse/DS-799

Summary

Usage of o-bolt-inline-list were replaced with bolt-component List as was recommended in the research ticket

Details

Places where inline-list object was updated with Bolt-component List:

  • form-icons.twig
  • event-detail-twig
  • 05-footer.twig

How to test

Check if there are no regressions due to the replacement inline-list object with the List component.

Release notes

Object inline-list is deprecated. Please use the Bolt component List.

@github-actions github-actions bot added the type: feature List this PR in the 'Features' section of the release notes. label Jul 14, 2022
@colbytcook colbytcook temporarily deployed to feature/DS-799-inline-list-deprecation--branch-preview July 14, 2022 15:01 Inactive
@@ -414,7 +414,7 @@ $bolt-input-transition: var(--bolt-transition);
top: 1px;
right: 1px;
padding-top: calc(
var(--bolt-spacing-y-medium) / 2 - 0.25rem
var(--bolt-spacing-y-medium) / 2.5 - 0.25rem
Copy link
Contributor

Choose a reason for hiding this comment

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

@MarcinMr is this change intended? I don't see any mention of it in the description so I wanted to double check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@colbytcook After replacing o-bolt-list with the list component I saw a small vertical misalignment of icons inside form input, this is why I adjusted it a little bit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MarcinMr minor suggestion: since we're already using one "magic number" here, .25rem, I would leave the divisor as 2 and tweak the .25rem value as needed.

Also, see Mike's comment on the next line and please update the comment with the new value. You can test the Japanese alignment by changing the [lang] tag on the <html> element to lang="ja".

Be sure to pull the latest before making updates. I resolved a conflict with the .incache file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@danielamorse thanks for the suggestion, I updated the numbers and comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Vertical alignment before changes
Old number

Vertical alignment after changes
New number

Copy link
Contributor

@colbytcook colbytcook left a comment

Choose a reason for hiding this comment

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

@MarcinMr approved with one minor comment

@colbytcook colbytcook temporarily deployed to feature/DS-799-inline-list-deprecation--branch-preview July 26, 2022 18:18 Inactive
@@ -414,7 +414,7 @@ $bolt-input-transition: var(--bolt-transition);
top: 1px;
right: 1px;
padding-top: calc(
var(--bolt-spacing-y-medium) / 2 - 0.25rem
var(--bolt-spacing-y-medium) / 2.5 - 0.25rem
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MarcinMr minor suggestion: since we're already using one "magic number" here, .25rem, I would leave the divisor as 2 and tweak the .25rem value as needed.

Also, see Mike's comment on the next line and please update the comment with the new value. You can test the Japanese alignment by changing the [lang] tag on the <html> element to lang="ja".

Be sure to pull the latest before making updates. I resolved a conflict with the .incache file.

@colbytcook colbytcook temporarily deployed to feature/DS-799-inline-list-deprecation--branch-preview July 27, 2022 17:28 Inactive
Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

Nice, thanks @MarcinMr!

@danielamorse danielamorse merged commit 74c8996 into master Jul 28, 2022
@danielamorse danielamorse deleted the feature/DS-799-inline-list-deprecation branch July 28, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature List this PR in the 'Features' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants