twinssbc / Ionic-Calendar

A calendar directive for Ionic framework
http://twinssbc.github.io/Ionic-Calendar/demo
MIT License
159 stars 73 forks source link

Ionic Grid System - col & row #17

Closed antoine92190 closed 8 years ago

antoine92190 commented 8 years ago

Hi, Thank you for sharing this awesome work! I see that you are using tables in your html. I played with your code and implemented the ionic grid system instead. I think it is simpler and way more flexible. Thus, rows and columns are evenly distributed. Do you think you could update your work and use the ionic grid system? I can help you with it if you want. In order to customize your calendar in a flexible way, I think it can be very useful. Kind regards, Antoine

twinssbc commented 8 years ago

@antoine92190 Thanks for your suggestion. I know that grid system is powerful, you can easily control the span and offset of a cell, but in this calendar scenario, what's the benefit can grid system bring comparing to a normal table?

antoine92190 commented 8 years ago

@twinssbc The problem is when you look closely at the month view, the columns’ width is not even. I’ll admit I care a lot about details, but that’s how you get an awesome product at the end of the day. For exemple in the month view, if I want to give a border-radius background-color around day numbers which contain an event, this issue becomes really obvious.

Another problem is that day, week and month views are defined under the same table properties. If I want to remove all borders from the month view (which I think is nice), I can’t because it would also remove borders in day and week views. I think rows and columns of each view should have their own property.

Using grid system can efficiently solve all those issues, and make your code much cleaner.

twinssbc commented 8 years ago

@antoine92190 Thanks for pointing out the issues. I like people who are "picky" to the product:) The reason I'm not keen on using grid system is because as you may know this calendar is coming from my another project which is a general Angular Responsive calendar. And I don't want to depend on other css framework such as Bootstrap, that's why I don't use the Bootstrap grid system either. I think these two issues can be solved by several lines of css styles, using table is not the real root cause. I will first try to fix them by adding some css styles, if I can't fix them in a clean way, then I will switch to grid system. Thanks.

antoine92190 commented 8 years ago

@twinssbc I understand, and I also don't like using bootstrap in ionic. But you did a great job by creating a specific Ionic version, so taking advantage of ionic features such as its own grid system is, in my opinion, what you are aiming for :) Anyway, as long as those issues are solved, any method is good! Thanks again for solving it and especially for your awesome work!!

twinssbc commented 8 years ago

@antoine92190 I have fixed the issue by just applying table-fixed style to the table. Feel free to have a test and let me know if any other place requires modification. Thanks.

antoine92190 commented 8 years ago

Thanks, it works nicely! There is only one other issue: day, week and month tables are using the same "table-bordered" property. For exemple, if I want to remove month borders, it also removes day and week borders. It would be nice to use different properties for each view, for exemple: table-bordered-day, table-bordered-week and table-bordered-month. Would you mind setting that up? And again, thank you so much!

twinssbc commented 8 years ago

@antoine92190 There are already other css classes applied to the tables. monthview-datetable, weekview-header, weekview-allday-content-table, etc. I will remove the table-bordered class and add the border to the above classes.

antoine92190 commented 8 years ago

Great thank you!

twinssbc commented 8 years ago

@antoine92190 I tried to move the style in table-bordered into other classes applied to the tables. I found it's not just a small change. I have some styles defined as below.

.table-bordered > thead > tr > th, .table-bordered > tbody > tr > th, .table-bordered > tfoot > tr > th,
.table-bordered > thead > tr > td, .table-bordered > tbody > tr > td, .table-bordered > tfoot > tr > td {
    border: 1px solid #ddd;
}

.table-bordered > thead > tr > th, .table-bordered > thead > tr > td {
    border-bottom-width: 2px;
}

If I replace table-bordered with 7 other classes (monthview-datetable, weekview-header, etc) one by one, the css size is increased a lot and I think it will also impact the performance as browser has more selector rules to check. If you only want to remove the month border, could you just override the style applied to month view table? For example,

.monthview-datetable
{
border: none !important;
}
. monthview-datetable > thead > tr > th, . monthview-datetable > tbody > tr > th, . monthview-datetable > tfoot > tr > th,
. monthview-datetable > thead > tr > td, . monthview-datetable > tbody > tr > td, . monthview-datetable > tfoot > tr > td {
    border: none !important;
}
antoine92190 commented 8 years ago

@twinssbc That does the trick! Thanks a lot

antoine92190 commented 8 years ago

@twinssbc Last quick question: are you planning to pull the "step" branch into master?

twinssbc commented 8 years ago

@antoine92190 yes, I will.

twinssbc commented 8 years ago

@antoine92190 The step branch has been merged to master, it's included in version 0.2.0.

hussainak commented 7 years ago

Hi @antoine92190 Are you able to share your grid system implementation of this calendar?