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

FRONT-2449: Button Pattern. #45

Merged
merged 10 commits into from
Sep 28, 2021
Merged

FRONT-2449: Button Pattern. #45

merged 10 commits into from
Sep 28, 2021

Conversation

tibi2303
Copy link
Contributor

No description provided.

@abel-santos-corral abel-santos-corral linked an issue Aug 25, 2021 that may be closed by this pull request
Copy link
Contributor

@drishu drishu left a comment

Choose a reason for hiding this comment

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

see todo in templates/overrides/form/input--submit.html.twig
this needs to be implemented.
Take notice of button attributes (primary secondary) and change according to the pattern variants.

Please add - "bcl" under file_scan_ignore_directories in runner.dist.yml until #61 is solved.

@escuriola
Copy link
Contributor

see todo in templates/overrides/form/input--submit.html.twig
this needs to be implemented.
Take notice of button attributes (primary secondary) and change according to the pattern variants.

Please add - "bcl" under file_scan_ignore_directories in runner.dist.yml until #61 is solved.

This would print a button which by itself that not submit the form.

@drishu
Copy link
Contributor

drishu commented Sep 7, 2021

see todo in templates/overrides/form/input--submit.html.twig
this needs to be implemented.
Take notice of button attributes (primary secondary) and change according to the pattern variants.
Please add - "bcl" under file_scan_ignore_directories in runner.dist.yml until #61 is solved.

This would print a button which by itself that not submit the form.

I think that if the button is of type submit then its fine, please test

Copy link
Contributor

@drishu drishu left a comment

Choose a reason for hiding this comment

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

I think for this task we should theme button elements also, I looked into it, to do this we can't override a template, but we can theme from scss
in _all.scss add

button.link {
  display: inline;
  margin: 0;
  padding: 0;
  cursor: pointer;
  border: 0;
  background: transparent;
  font-size: 1em;
  text-decoration: underline;
  appearance: none;
}

this is needed so that the "Edit summary" button looks ok in create content form (and other forms )

Copy link
Contributor

@drishu drishu left a comment

Choose a reason for hiding this comment

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

Looks good, but one more hurdle to pass, the functionality of other patterns was affected because the fields and options changed, please fix:
pattern-collapse.html.twig
pattern-dropdown.html.twig
pattern-modal.html.twig
pattern-popover.html.twig

@Maxfire
Copy link
Contributor

Maxfire commented Sep 24, 2021

Looks good, but one more hurdle to pass, the functionality of other patterns was affected because the fields and options changed, please fix:
pattern-collapse.html.twig
pattern-dropdown.html.twig
pattern-modal.html.twig
pattern-popover.html.twig

I checked all and collapse was fixed, also checked different variants, greetings.

{% if _icon %}
{% set _icon = icon|merge({
path: bcl_icon_path
}) %}
Copy link
Contributor

Choose a reason for hiding this comment

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

intendation is not correct, and there is a extra space

Suggested change
}) %}
}) %}

{#
/**
* @file
* Button component.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Button pattern.

@@ -10,5 +10,13 @@
* @see template_preprocess_input()
*/
#}
{# @TODO Revisit after refactoring the button pattern. #}
<input{{ attributes.addClass(['btn', 'btn-primary']) }} />{{ children }}
{% if attributes.hasClass('button--primary') %}
Copy link
Contributor

Choose a reason for hiding this comment

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

in the login form, the class button--primary is not set for the login button, it seems that this class is added per use case and not automatically by a rule.
To solve this issue, we can do something like {% elseif attributes['data-drupal-selector'] == 'edit-submit' %}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I'm going to add it as or condition in the same if. thanks.

* Button component.
*/
#}
{% if _icon %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% if _icon %}
{% if icon %}

{% set spinner = {
'type': "border",
'variant': "info",
'small': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

the icon is to close to the text, please add attributes: create_attribute().addClass('me-1'), like in netlify data tab

type: text
label: Assistive text
description: Additional text hidden with the .visually-hidden class.
preview: ''
url:
Copy link
Contributor

Choose a reason for hiding this comment

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

url doesn't exist

@@ -18,13 +18,13 @@
label: label,
style: style,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
style: style,
variant: style,

Copy link
Contributor

Choose a reason for hiding this comment

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

this could be a bit confusing but ok.

@escuriola escuriola merged commit de3db78 into 1.x Sep 28, 2021
@drishu drishu deleted the FRONT-2449 branch September 28, 2021 14:06
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.

Refactor Button pattern with BCL component
5 participants