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

Explicitly transmit codeblock offset accross workflow "Show => Section edit => Preview". #2204

Closed
wants to merge 2 commits into from

Conversation

lpaulsen93
Copy link
Collaborator

This helps finding the correct codeblock if a code download is triggered in preview mode. Fixes #741.

…n edit => Preview".

This helps finding the correct codeblock if a code download is triggered in preview mode. Fixes #741.
inc/html.php Outdated
@@ -126,6 +126,10 @@ function html_secedit_button($matches){
$data['name'] = $matches['name'];
}

if (!empty($matches['codeblockOffset'])) {
$data['codeblockOffset'] = strtolower($matches['codeblockOffset']);
Copy link
Collaborator

Choose a reason for hiding this comment

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

strtolower() seems unnecessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, copy&paste leftover...changed it.

@splitbrain
Copy link
Collaborator

In general I'm fine with this PR, however I think the way we markup the sections gets a bit complicated. I think it would be time to refactor this a bit:

  • Instead of writing a single string with lots of optional parts that has to be parsed by a complicated regexp, we could simply write out a JSON array
  • Instead of passing 5 variables to startSectionEdit() we could pass a single associative array
  • Instead of storing a list of indexed arrays in $this->sectionedits we could store the same associative array

When implementing this, we need to check if plugins implementing a custom editor (data, edittable, ...) need any adjustments.

@lpaulsen93
Copy link
Collaborator Author

I will leave this as is and handle the refactoring in branch sectioneditrefactor.

@splitbrain splitbrain added this to the 🐱 Greebo milestone Jan 24, 2018
@splitbrain
Copy link
Collaborator

I guess we can close this in favour of #2220?

@lpaulsen93
Copy link
Collaborator Author

Yes! I'll close it now.

@lpaulsen93 lpaulsen93 closed this Jan 24, 2018
@lpaulsen93 lpaulsen93 deleted the issue741 branch September 15, 2018 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants