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

Add bootswatch theme specific tip box styles #999

Closed
wants to merge 2 commits into from

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented Jan 26, 2020

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Enhancement to an existing feature

Resolves #962

Accompanying pull request to MarkBind/vue-strap#118

What is the rationale for this request?
To add tweak tip box heading styling and add theme specific styles for said headings, as good looking headings is difficult to achieve on all themes with generic styling.

What changes did you make? (Give an overview)

  • Added bootswatch theme specific styles for tip boxes
  • Some code to copy this additional stylesheet 'layer' when building the site

Preview (uses theme's alert-xx color)
plainTipBoxStyle

Preview (theme-specific color):

bootswatchTipBoxStyles

Provide some example code that this change will affect:

// Under site.js, does the same as getBootswatchThemePath
function getExtraBootswatchThemePath(theme) {
  return path.join(__dirname, 'lib', 'bootswatch', 'src', theme, 'bootstrap-markbind.css');
}

// Under site.js SUPPORTED_THEMES_EXTRA_PATHS constant
...
'bootswatch-cerulean': getExtraBootswatchThemePath('cerulean'),
'bootswatch-cosmo': getExtraBootswatchThemePath('cosmo'),
'bootswatch-flatly': getExtraBootswatchThemePath('flatly'),
...

Is there anything you'd like reviewers to focus on?
The implementation of theme specific styles ( #997 ),
mainly whether bootswatch should be patched directly.

Testing instructions:

  1. Build the dist from Enhance box component heading vue-strap#118
  2. Use the built dist and execute markbind serve -d to launch the template page with all the variants of the tip boxes.
  3. Use the style switcher component under heading 2 to switch between the themes quickly for testing ( will be removed from the PR if the styles are ok )

styleswitcher

  1. npm run updatetest should pass ( number of assets generated unit test is changed, and expected test files )

Proposed commit message: (wrap lines at 72 characters)
Add bootswatch theme specific tip box styles

@ang-zeyu ang-zeyu force-pushed the box-heading-theme-styles branch 4 times, most recently from e931060 to dd5237a Compare January 26, 2020 16:07
@@ -0,0 +1,22 @@
.alert-warning-darker {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the attempt, but we really shouldn't do this. It's a lot of effort for very little value.
My thoughts on styling: #386 (comment), #386 (comment), MarkBind/vue-strap#124 (comment)

Copy link
Contributor Author

@ang-zeyu ang-zeyu Jan 27, 2020

Choose a reason for hiding this comment

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

Noted 👍

Hmm, shall we go with the barebones approach mentioned in #962 then? @damithc @acjh

It's plain, but still looks good imo, and dosen't require theme specific styling
I'll edit the main post with a new gif in a while

@ang-zeyu
Copy link
Contributor Author

ang-zeyu commented Feb 4, 2020

As discussed with @damithc, we'll simply go with the alert-theme-xx's color for easier maintainability.

I'll close this PR and make a new one for posterity

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.

x of the dismissible boxes it not at the top-right corner
2 participants