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

fix: refactor Action Bar title to follow the latest fund-styles #3466

Merged
merged 2 commits into from
Sep 30, 2020

Conversation

InnaAtanasova
Copy link
Contributor

@InnaAtanasova InnaAtanasova commented Sep 29, 2020

Please provide a link to the associated issue.

Part of #3420

Please provide a brief summary of this pull request.

In latest release of fundamental-styles the markup for Action Bar title is changes, also an additional class is added. The fd-action-bar-title directive is now a component.

BREAKING CHANGE:
fd-action-bar-title directive is now a component.

Before:

<h3 fd-action-bar-title>Page Title</h3>

After:

<div fd-action-bar-title>Page Title</div>

Please check whether the PR fulfills the following requirements

Documentation checklist:

@netlify
Copy link

netlify bot commented Sep 29, 2020

Deploy preview for fundamental-ngx ready!

Built with commit c11598d

https://deploy-preview-3466--fundamental-ngx.netlify.app

@InnaAtanasova InnaAtanasova added the core Core library specific issues label Sep 30, 2020
Copy link
Contributor

@JKMarkowski JKMarkowski left a comment

Choose a reason for hiding this comment

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

Let's merge it and discuss about refactor after release

Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

Why don't we use it like a custom element:

Like: 
<fd-action-bar-title> Page Title </fd-action-bar-title>

Instead of:
<div fd-action-bar-title> Page Title </div>

div tag does not provide any additional semantic meaning, and I feel like it a bit complicating the markup.

@salarenko
Copy link
Contributor

Lets discuss the directives later.

@droshev droshev self-requested a review September 30, 2020 23:15
@droshev droshev merged commit 5d80c05 into master Sep 30, 2020
@droshev droshev deleted the fix/action-bar-refactor-latest-styles branch September 30, 2020 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core library specific issues Defect Hunting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants