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

Blockbase comments block refactor #4535

Merged
merged 8 commits into from
Sep 8, 2021
Merged

Conversation

MaggieCabrera
Copy link
Contributor

@MaggieCabrera MaggieCabrera commented Sep 7, 2021

Changes proposed in this Pull Request:

This PR refactors the Post comments block for Blockbase and all its children according to what's discussed on #4515 and #4531
There was some redundant CSS on some of the themes and some that could be refactored into Blockbase to work for all the children.

This is the before and after:

Blockbase:

Before After
Screenshot 2021-09-07 at 12-53-54 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 12-53-32 Gutenberg Verse – Skatepark

Seedlet:

Before After
Screenshot 2021-09-07 at 12-56-46 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 12-56-33 Gutenberg Verse – Skatepark

Mayland:

Before After
Screenshot 2021-09-07 at 12-55-21 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 12-54-57 Gutenberg Verse – Skatepark

Quadrat:

Before After
Screenshot 2021-09-07 at 12-57-08 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 12-57-23 Gutenberg Verse – Skatepark

Geologist:

Before After
Screenshot 2021-09-07 at 12-59-13 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 12-59-26 Gutenberg Verse – Skatepark

Skatepark:

Before After
Screenshot 2021-09-07 at 12-58-26 Gutenberg Verse – Skatepark Screenshot 2021-09-07 at 13-07-48 Gutenberg Verse – Skatepark

Related issue(s):

Closes #4515
Partially addresses #4531

@MaggieCabrera MaggieCabrera marked this pull request as ready for review September 7, 2021 11:10
@MaggieCabrera MaggieCabrera self-assigned this Sep 7, 2021
@MaggieCabrera MaggieCabrera requested review from melchoyce, beafialho, kjellr and a team September 7, 2021 11:10
@kjellr
Copy link
Contributor

kjellr commented Sep 7, 2021

This looks great! Just a few comments:

  1. On second thought, let's make the font size of the reply link smaller. I suggest we match the font size of the comment meta (0.75em).
  2. Please make sure all of the Reply links are underlined.
  3. On Skatepark, it looks like the comment header isn't lining up correctly with the avatar. See the screenshot below.
  4. Seedlet doesn't appear to be indenting its comments properly (this is noticeable in your screenshot above too).
Skatepark (Broken) Quadrat (Correct)
Screen Shot 2021-09-07 at 8 52 11 AM Screen Shot 2021-09-07 at 8 54 07 AM

Seedlet, without nesting:

Screen Shot 2021-09-07 at 8 52 45 AM

Thank you!

@MaggieCabrera MaggieCabrera force-pushed the blockbase-comments-refactor branch from 8329da5 to 2118ae6 Compare September 7, 2021 14:42
@MaggieCabrera
Copy link
Contributor Author

This looks great! Just a few comments:

1. On second thought, let's make the font size of the reply link smaller. I suggest we match the font size of the comment meta (`0.75em`).

2. Please make sure all of the Reply links are underlined.

3. On Skatepark, it looks like the comment header isn't lining up correctly with the avatar. See the screenshot below.

4. Seedlet doesn't appear to be indenting its comments properly (this is noticeable in your screenshot above too).

Thanks so much for the quick review! I think I fixed all the points above, this is ready for another review.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

This looks great, thanks Maggie! We may refine more eventually, but this is a great starting point for these, and it's way nicer than what we had before. 🙌

One very tiny thing I noticed in Seedlet Blocks: The spacing above and below each comment should be equal, but right now there's more space at the bottom than at the top:

Screen Shot 2021-09-07 at 3 45 09 PM

Feel free to fix that here or in a followup PR! Thank you!

@MaggieCabrera MaggieCabrera force-pushed the blockbase-comments-refactor branch from 89975f0 to d738ee5 Compare September 8, 2021 07:35
@MaggieCabrera
Copy link
Contributor Author

Fixed the last thing and bringing this in

@MaggieCabrera MaggieCabrera merged commit 012baa2 into trunk Sep 8, 2021
@MaggieCabrera MaggieCabrera deleted the blockbase-comments-refactor branch September 8, 2021 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blockbase: Refine comments styling
2 participants