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

Added bootstrap4 theme #4065

Merged
merged 2 commits into from Mar 4, 2018

Conversation

Projects
None yet
4 participants
@GeekJosh

GeekJosh commented Feb 19, 2018

This PR adds support for a bootstrap4 theme. I have taken the approach of using FontAwesome for icons as this remains freely available.

Josh Johnson Josh Johnson

@GeekJosh GeekJosh referenced this pull request Feb 19, 2018

Closed

Bootstrap 4 Theme #4032

@acerix

This comment has been minimized.

Member

acerix commented Feb 19, 2018

Thanks for the PR

@acerix acerix added the Theme label Feb 19, 2018

@joankaradimov

Thanks for implementing this. I'm looking forward to using it.

I'd like to suggest a couple of changes...

// day grid
headerRow: 'border-primary', // avoid `panel` class b/c don't want margins/radius. only border color
dayRow: 'border-primary', // "

This comment has been minimized.

@joankaradimov

joankaradimov Feb 20, 2018

Contributor

In the day/week views there is some weird coloring in the top-right corner of the table. Although this is a bit hacky, if you replace this border-primary with table-bordered, the color would match the surrounding borders.

<option value='spacelab'>Spacelab</option>
<option value='superhero'>Superhero</option>
<option value='united'>United</option>
<option value='yeti'>Yeti</option>

This comment has been minimized.

@joankaradimov

joankaradimov Feb 20, 2018

Contributor

The list of themes in Bootswatch has changed, it appears.

These two no longer exist:

  • paper
  • readable

These are new ones that can be added to this list:

  • cerulean
  • litera
  • lux
  • materia
  • minty
  • pulse
  • sketchy
.fc-bootstrap4 .fc-time-grid .fc-slats table {
/* some themes have background color. see through to slats */
background: none;
}

This comment has been minimized.

@joankaradimov

joankaradimov Feb 20, 2018

Contributor

If you go to the month view in the demos, there will be a day with a long list of events. They are hidden and a text like +N more appears. When I clicked it a card appeared outside the table. I needed to add a little CSS to fix it...

.fc-bootstrap4 .card {
  position: absolute;
}

.... maybe the selector can be further refined. This might be too generic.

nextYear: 'fa-angle-double-right'
}
Bootstrap4Theme.prototype.iconOverrideOption = 'bootstrapGlyphicons'

This comment has been minimized.

@joankaradimov

joankaradimov Feb 20, 2018

Contributor

I don't recall how bootstrapGlyphicons worked. But it is supposed to allow you to not use Glyphicons with Bootstrap 3. Maybe in the case of Bootstrap 4 it makes sense to have something called bootstrapFontAwesome... and then have some similar logic to Bootstrap 3.

This comment has been minimized.

@GeekJosh

GeekJosh Feb 23, 2018

That's great feedback, thank you. I'll update my PR later on today

This comment has been minimized.

@joankaradimov

joankaradimov Feb 23, 2018

Contributor

Thanks!

@GeekJosh

This comment has been minimized.

GeekJosh commented Feb 23, 2018

@joankaradimov

  • I've updated the list of themes, thanks for listing them for me
  • The colour fix using table-bordered works well, this is what the outer container is using so the colour will always match
  • I've added some extra styling around popovers so they should all e fixed now (Bootstrap overrides the position set by fullcalendar, so I've added a more specific CSS selector to retain absolute
  • As suggested, icon overrides are now done using a bootstrapFontAwesome so that it doesn't conflict with the bootstrap3 theme
@GeekJosh

This comment has been minimized.

GeekJosh commented Feb 23, 2018

The tests are failing on the repo due to an issue in locale-all - this is happening on the unmodified repo too. All tests for what I have added appear fine though (I have also added a manual test)

@arshaw arshaw added this to the next-release milestone Mar 1, 2018

@arshaw arshaw merged commit 965078d into fullcalendar:master Mar 4, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@arshaw

This comment has been minimized.

Member

arshaw commented Mar 4, 2018

really high quality PR and great feedback! i've merged this, made some tweaks, and will queue for release

@arshaw

This comment has been minimized.

Member

arshaw commented Mar 5, 2018

released in v3.9.0 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment