Author = Byline component

Rain-kanyini commented 1 year ago

Summary

This created a Byline-Meta Component. Since byline is outdated, I changed the markup to use an section tag rather than byline. Should this still be called a "byline" component?

This PR fixes/implements the following bugs/features

This creates a new component for Storybook, which allows for a byline to be used

How to review this PR

[x] Go Here
[x] Click on Text -> Byline Example
[x] View this component to see if there are any changes that need to be made to it Closing issues

Should this still be called a "byline" component?

Rain-kanyini commented 1 year ago

Link to previous comments that Callin left

https://github.com/emulsify-ds/compound/pull/48#pullrequestreview-1099291321

Rain-kanyini commented 1 year ago

I want to preserve the notes on the previous branch, if this looks good. I will delete the other branch :) @callinmullaney

Rain-kanyini commented 1 year ago

Callin's comments on 09-09-2022

callinmullaney 14 days ago @Gregg-Hendrix

Add the logic to handle the date+time formatting. Currently, this is just set as TODO
Add single quotes around the datetime and class attribute names

After chatting with Brian and JoeT about some of the variable documentation we've been doing we've decided that it's not worth add comments for __base_class, modifiers, and *blockname. These variables should be a given with any component as we use BEM class structure for everything. Can you remove these comments?

[components/01-atoms/text/text/07-byline.twig](https://github.com/emulsify-ds/compound/pull/48/files/e9d725304fb6253e07313e1c6f46101fdf8e10af#diff-c63c42e3869900036f94deed202bb6bed237a387f8b96bca1266d97636c63fb3)

{% set byline__base_class = byline__base_class|default('byline') %}

{# {% set date_time__attributes = {
Tip
Member
@[callinmullaney](https://github.com/callinmullaney) callinmullaney [14 days ago](https://github.com/emulsify-ds/compound/pull/48#discussion_r964926449)
Is this commented code necessary? Can we remove it or were you planning on utilizing it here somehow?

[components/01-atoms/text/text/07-byline.twig](https://github.com/emulsify-ds/compound/pull/48/files/e9d725304fb6253e07313e1c6f46101fdf8e10af#diff-c63c42e3869900036f94deed202bb6bed237a387f8b96bca1266d97636c63fb3)

  <byline {{ add_attributes(byline__attributes) }}>
    {% block byline_content %}
      {{ byline_content }}
Tip
Member
@[callinmullaney](https://github.com/callinmullaney) callinmullaney [14 days ago](https://github.com/emulsify-ds/compound/pull/48#discussion_r964938711) • 
This feedback also applies to the comments above: We should be using double underscores to separate different parts of a component and single underscores when it's suppose to act as a space within a name of a variable. So the naming convention should be something like [component_name]__[component_part].

Example {{ byline__author }} indicates ownership of this variable to the byline component and author is the specific value we want to display. This could also be extrapolated to {{ byline__first_author }} and {{ byline__second_author }} <-- this example follows the same naming convention but shows that first_author and second_author are specific values and not separate. Taking this naming convention even further {{ byline__first_author__image }} This variable name indicates this is a byline specific variable, a first_author element/value is expected, and image indicates we want to print an image.
point_up hope I explained the reasoning behind the naming convention but let me know if you want to chat further.

I think it would be worth researching this element a little more and figure out if this is the most semantic way to markup author type content. <byline> seems like it was an HTML spec back in HTML2 but doing a quick google search on <byline> is coming up with much.

@[Gregg-Hendrix](https://github.com/Gregg-Hendrix) Gregg-Hendrix Pending
@callinmullaney , thank you so much for this feedback! You explained it really well, embarrassingly I'm still a bit confused but I'm going to try to put this in my own words, because I may understand this !

byline__author

Default class = Byline

BEM class within the byline = Author

To style this it would be byline_author

If we wanted multiple authors.... We would put a double underscore after byline? This is what confuses me. I feel like it should be byline_author__first ? However you have byline__first_author ? I can learn to understand this, but I am slightly confused.

If I'm still confused on Wednesday, I'd love to go over it on our 1 on 1 (I'll also do research on my own about where/when to place double underscores vs single). I understand that your solution works, and I can replicate that, I'm just like "hmmmm" how does it work?

@Gregg-Hendrix if this PR is no longer valid can you capture the comments here, address them in your other PR, and close this PR with a comment to why it was closed? thanks!

amazingrando commented 5 months ago

Merge conflicts have been resolved.

sync-by-unito[bot] commented 5 months ago

➤ Fabiola Fallas Cascante commented:

Hi @Marvin J. Cortés @Randy Oest this issue was first solved by Rain, we need to reassign it.

randy commented 5 months ago

➤ Fabiola Fallas Cascante commented:

Hi @marvin J. Cortés @randy Oest this issue was first solved by Rain, we need to reassign it.

@amazingrando Is this config on your end or Unito's? I don't mind reporting a bug...

amazingrando commented 5 months ago

@randy Our PM has been working with Unito to get some things sorted, this old message got posted here, and you got tagged.

My username in Clickup is no longer "Randy Oest", so new messages won't tag you. Obviously old messages that get processed again by Unito will tag you.
You can submit a ticket to Unito about this, asking them to either 1) use a full tag name from Clickup (e.g. "Randy Oest"), or 2) prevent posts to Github from tagging people

emulsify-ds / compound

Author = Byline component #58