Skip to content

Feature/86 update custom containers #90

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

Merged
merged 3 commits into from
May 15, 2020

Conversation

bencodezen
Copy link
Member

The styles are temporarily in a global stylus file and needs refactoring, but until the atomic theme is ready, wanted to split this off to a different PR so people could get access and start using this feature sooner rather than later.

Screen Shot 2020-05-09 at 6 15 08 AM

Also, the icons are completely fill-ins and open to us figuring out what icon / symbols we want for each custom container box. And as always, totally open to changes / new style ideas. I mainly migrated existing styles from the current docs.

@phanan
Copy link
Member

phanan commented May 9, 2020

Hmm… To be honest, I'd prefer to keep the current style as-is, as it already has two elements to explain the intention: color and text/label. The icons don't add anything new IMHO, and instead are prone to unnecessary aesthetical discussions/bikeshedding.

@shentao
Copy link
Member

shentao commented May 9, 2020

Agree with An on the icons.

@bencodezen
Copy link
Member Author

Since we are planning on a redesign at some point, the idea is to minimize the amount of change that people have to deal with while allowing contributors more ways of breaking up information.

@phanan
Copy link
Member

phanan commented May 11, 2020

I'm not sure I follow. We already have these components right?

@bencodezen
Copy link
Member Author

bencodezen commented May 11, 2020

Unless I'm mistaken, the existing docs we have the tip and danger container equivalents, but don't have the other ones.

As far as the types go, the naming / color association with the custom containers tend to imply certain things:

  • Tip (Green): This is something good to do and basically recommended
  • Info (Blue): Neutral information that is worth noting but has neither positive or negative implications
  • Warning (Yellow): This is information we think should watch out for, but if you choose to ignore this, it's up to you
  • Danger (Red): This is bad and we need you to pay attention to think or else you'll regret it later

Do we already have all of these in the existing docs?

@phanan
Copy link
Member

phanan commented May 11, 2020 via email

@bencodezen
Copy link
Member Author

@phanan Icons have been removed except for the danger to maintain consistency. The styles will update accordingly once I've migrated the existing site's CSS over to an atomic VuePress theme.

@bencodezen bencodezen merged commit 6275464 into master May 15, 2020
@sdras
Copy link
Member

sdras commented May 16, 2020

I'm sorry I'm late to the party here, I disagree with removing the icons. We talked through in a few meetings and with Evan that we needed more than one style for information, danger and the like.

  • Icons can help with cognitive leaks because the user can immediately identify what kind of information they're seeing, there are studies that show that the use of icons can help a person navigate a site faster and understand meaning quicker

I would love to talk this through. @phanan is there a meeting you can attend? I know it's tough and I can't always join either, but would be good to get comms in one place :)

@phanan
Copy link
Member

phanan commented May 16, 2020

Sure, I'll try to attend the next meetings. Re: the icons, I do agree that icons can help with cognitive leaks. In this case, however, I'd argue that it's superfluous for the same reason above: labels and colors. Having three elements to convey one message to me is an overkill.

@sdras
Copy link
Member

sdras commented May 16, 2020

Just so I understand, are you proposing to kill the warning one and keep the others? That's the only one I see that's repetition. The other labels we decided to add.

TalexDreamSoul pushed a commit to Talexs/docs that referenced this pull request Apr 17, 2022
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.

5 participants