Skip to content

Support for inserting an item in a CSSList #115

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

Closed
wants to merge 1 commit into from

Conversation

FMCorz
Copy link

@FMCorz FMCorz commented Aug 1, 2016

For your consideration.

FMCorz pushed a commit to FMCorz/moodle that referenced this pull request Aug 3, 2016
danpoltawski pushed a commit to danpoltawski/moodle that referenced this pull request Sep 23, 2016
Chocolate-lightning added a commit to Chocolate-lightning/moodle that referenced this pull request Jun 20, 2019
@raxbg
Copy link
Contributor

raxbg commented Nov 13, 2019

I like the idea, but the method name insert suggests that it must have a parameter for position: before or after. Also what if we just map this to the existing replace? Suggestion:

	public function insert($oItem, $oSibling, $sPosition = 'before') {
		$iIndex = array_search($oSibling, $this->aContents);
		if ($iIndex === false) {
			return $this->append($oItem);
		} else if ($sPosition === 'before') {
			$this->replace($oItem, array($oSibling, $oItem));
		} else {
			$this->replace($oItem, array($oItem, $oSibling));
		}
	}

@oliverklee oliverklee deleted the branch MyIntervals:main February 7, 2024 11:36
@oliverklee oliverklee closed this Feb 7, 2024
@oliverklee oliverklee reopened this Feb 7, 2024
@oliverklee oliverklee changed the base branch from master to main February 7, 2024 22:38
@ziegenberg
Copy link
Contributor

As there was no progress on this for the last five years I created #545.

@ziegenberg
Copy link
Contributor

@oliverklee I think this can now be closed.

@oliverklee
Copy link
Collaborator

Indeed it can. Thanks for implementing/finishing this, @ziegenberg! ❤️

Implemented in #545.

@oliverklee oliverklee closed this Jun 20, 2024
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.

4 participants