Handle dynamic document titles #19

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@ipmb

ipmb commented Mar 8, 2012

Adds fallbackMethod option for dynamic document titles

  • static is the traditional behavior based on the original document title
  • dynamic uses a regex to replace the title in the event the original document title changed since the script loaded

ipmb added some commits Mar 8, 2012

Adds `fallbackMethod` option for dynamic document titles
* `static` is the traditional behavior based on the original document title
* `dynamic` uses a regex to replace the title in the event the original document title changed since the script loaded
@tommoor

This comment has been minimized.

Show comment
Hide comment
@tommoor

tommoor Mar 10, 2012

Owner

Hey, can you think of any reason why you would want to use the static method? I'm reluctant to add extra options unless really necessary :-)

Owner

tommoor commented Mar 10, 2012

Hey, can you think of any reason why you would want to use the static method? I'm reluctant to add extra options unless really necessary :-)

@ipmb

This comment has been minimized.

Show comment
Hide comment
@ipmb

ipmb Mar 12, 2012

This isn't fool-proof. If you started with a page title in the form (20) Some Text it would overwrite the (20) part. You'd need to be smarter about handling the fallback for these instances, probably maintaining some internal state.

ipmb commented Mar 12, 2012

This isn't fool-proof. If you started with a page title in the form (20) Some Text it would overwrite the (20) part. You'd need to be smarter about handling the fallback for these instances, probably maintaining some internal state.

@simong

This comment has been minimized.

Show comment
Hide comment
@simong

simong Jan 20, 2014

Contributor

Any progress on this? I'm willing to help out...

Contributor

simong commented Jan 20, 2014

Any progress on this? I'm willing to help out...

@simong

This comment has been minimized.

Show comment
Hide comment
@simong

simong Jan 21, 2014

Contributor

FWIW, I don't see any reason why their needs to be a static v dynamic distinction at all.
The fact that updateTitle doesn't take the current title into account feels like a bug and can be fixed without worrying about backwards compatibility (IMHO) in a major release.

I'd be willing to submit a new PR for that logic if there is interest ..

Contributor

simong commented Jan 21, 2014

FWIW, I don't see any reason why their needs to be a static v dynamic distinction at all.
The fact that updateTitle doesn't take the current title into account feels like a bug and can be fixed without worrying about backwards compatibility (IMHO) in a major release.

I'd be willing to submit a new PR for that logic if there is interest ..

@tommoor

This comment has been minimized.

Show comment
Hide comment
@tommoor

tommoor Jan 21, 2014

Owner

Agree with @simong, whilst this would be a great improvement - there shouldn't need to be a distinction and the lib should just be able to handle this situation.

Owner

tommoor commented Jan 21, 2014

Agree with @simong, whilst this would be a great improvement - there shouldn't need to be a distinction and the lib should just be able to handle this situation.

simong added a commit to simong/tinycon that referenced this pull request Jan 21, 2014

@simong

This comment has been minimized.

Show comment
Hide comment
@simong

simong Jan 21, 2014

Contributor

I've submitted a PR at #57 which addresses the issue directly.

Contributor

simong commented Jan 21, 2014

I've submitted a PR at #57 which addresses the issue directly.

@tommoor

This comment has been minimized.

Show comment
Hide comment
@tommoor

tommoor Feb 10, 2014

Owner

Closed by @simong in 9cc67d6, this implementation was preferred as it requires no extra work on behalf of the user.

Owner

tommoor commented Feb 10, 2014

Closed by @simong in 9cc67d6, this implementation was preferred as it requires no extra work on behalf of the user.

@tommoor tommoor closed this Feb 10, 2014

tiff added a commit to protonet/tinycon that referenced this pull request Mar 6, 2014

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