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

Added Feature to Hide the Header #26

Closed
wants to merge 8 commits into from

Conversation

traeblain
Copy link
Contributor

This allows the header to be set as false and therefore hiding the header but still having the data like title and subtitle.

Addresses part 1 of #21

Object.assign

Used Object.assign to default the header property to true. Right now it is inline, but I could see if you wanted to have some set of default settings in an object then merge the two, that could be done. It's up to you.

Did not set this in the data.config to start with because it would 1) initiate the app at all times since there's a v-if on that template portion. 2) I don't recall if Vue reactivity is triggered with Object.assign.

@bastienwirtz
Copy link
Owner

Thanks again for your contribution @traeblain!

There is a v-if on the whole config because nothing make sense without it, I see it as a requirements. What I would do is just add a the header: true in the config file, it's clean and simple, and enough a default value for me. I agree that if someone wants a dry config file and removes it, it will default to false, but that's fine.
Enforcing default value with object.assign is a good idea but not really necessary at this point, and would need to be done for all config root config key to be consistent and be documented.

Does the simple header: true in the config works for you?

@traeblain
Copy link
Contributor Author

I checked again. I thought if header was undefined the entire app won't initiate due to the v-if. So that's why I did what I did so people don't upgrade and you get Issues stating: "What happened!? I upgraded and it's broken!?" My goal was to provide something that was backwards compatible.

I agree the simplest approach (and aligning with your KISS mentality with this whole thing) is to simply have header: true in the config. May still get some "Where's my header?" issues, but easy to answer.

This should do it...although not sure how clean you want the git history. (My git skills for clean merges aren't very good.)

@traeblain
Copy link
Contributor Author

Hope I rebased this properly...the change is so simple, if I need to dump it for a new one let me know...

Honestly, probably didn't do it right...

@bastienwirtz
Copy link
Owner

Hey @traeblain
Same here, sorry again to have left you unanswered!
Your work on this has also been integrated to #62, thanks a lot!

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.

None yet

3 participants